Types breakage in v6.4.0

[Rugvip]

Types breakage in v6.4.0

[fpapado]

@Rugvip, the fix to this should now be released as part of 6.4.2 👀 Can you check on your end? :)

[Rugvip]

@fpapado Thank you for the reply! 🎉

1. Yes, if I switch it to just type ByRoleMatcher = ARIARole | (string & {}) I no longer see any errors.

I played around with the types a bit in this repo. I didn't find any instructions for how to check types, but I've been running npm exec tsc. Something seems strange with the setup in this repo. If I for example add something obviously broken like export 'bad' at the end of matchers.d.ts, I get an error. But even something like switching export = matchers to export = nonexistent doesn't break the type checking.

I moved over to reproducing in a minimal Backstage setup instead:

npx @backstage/create-app@latest --skip-install # "repro-jest-dom" as app name
cd repro-jest-dom
yarn install
yarn tsc:full # error

In that setup I could verify that removing the ByRoleMatcher export at least make those type checks happy.

[Rugvip]

@fpapado yep those do catch the export = notAThing, but as far as I can tell it's because the matchers are of the things explicitly tested. If you for example add type non = existent at the bottom that's not caught.

[fpapado]

Aha! I believe it is "skipLibCheck": true in the root tsconfig.json that is causing these errors to be silently ignored.

skipLibCheck skips checking all .d.ts files, unless specifically referenced by the source code. This explains the discrepancy that we are seeing between tests and running tsc directly 💡

By switching it off, or by running tsc directly on the types/matchers.d.ts file (i.e. bypassing tsconfig.json), the original error shows up, without the changes in #575.

However, when changing this, there is a cascade of other errors that show up. I do not know which ones are valid, because the type overloading / declaration merging in this library seems complex. This leads me to think that skipLibCheck is somehow load-bearing for the type setup here, but at least this gives a way to verify the changes!

Incidentally, at work, we also have "skipLibCheck": true, which would explain why I could not reproduce this originally.

CleanShot.2024-01-31.at.08.57.58.mp4

[pjungermann]

I tried it in our setup, and the fix works. Thank you!

[Rugvip]

Works for us too, thank you! 👍

[fpapado]

Hi @Rugvip! I am not a maintainer, but I am the person that contributed .toHaveRole, so I take some of the responsibility 😌 First of all, my apologies for the hassle!

Two quick checks

Can I ask you to check two things on your end?

1. If you modify matchers.d.ts to remove the export declaration for ByRoleMatcher, are the type errors fixed?
2. If not, can you check removing the import and export for ByRoleMatcher, and changing this line to role: string?

Since the core export of the package is the matchers, there is no need to separately export the ByRoleMatcher anyway. We just want the argument to .toHaveRole to be well-typed (the full type of the matcher can be extracted with Parameters/ReturnType and co if need be, I suppose) So option 1 might be a simple way to thread the needle, and a follow-up PR :)

You can use patch-package, pnpm's native patching, or even the code editor in a pinch, whichever you prefer.

I ask you, because I tried to reproduce this issue in our codebase at work, and we are not getting this issue. Tests type-check both with and without the change in option 1 above. TypeScript config is vast, so I'm not drawing any conclusions yet 😅

Some more context on the fixes

Might be enough to switch the re-export to export { matchers }?

This would probably not be enough. For example, the type tests in this repository fail with that change.

You can run the type tests in the repository via npm run test:types. The type tests themselves are in the types/tests directory.

I tried copying over your tsconfig.json into that tests folder, and the tests seemed to still pass. At a glance, I don't see anything in the config that could cause this, so I suspect some other underlying cause.

Future testing

I have a gut feeling that the type tests in this repository are missing some use-cases (perhaps config combinations), if they can break like this. I don't have a concrete suggestion for it right now; maybe the maintainers have run into this before?

[fpapado]

Thanks for checking, @Rugvip!

I opened a PR for the maintainers to consider, but I do not have a good test case to add to the repo. So I leave it up to them, how to proceed. I opened the PR mostly to account for any timezone differences, not because it is the end-all-be-all solution :)

But even something like switching export = matchers to export = nonexistent doesn't break the type checking.

Did you try running the type tests with npm run test:types? There's different tsconfig projects under types, so they are run individually, via that script. When I do that with a nonsensical export = notAThing, they seem to error as intended:

CleanShot.2024-01-30.at.21.10.03.mp4

[SantoJambit]

FYI, using skipLibCheck will cause type-errors to silently result in any types, so it's not a good idea to set this unless you really really need to.