-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adopt typescript #445
Comments
13 tasks
2 tasks
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
Bumped from TS 4.9 to 5.7. TS 5.8 released a couple of days ago and is not yet properly supported (via peerDependencies) in our other tooling, so we hold off on that for a while. @formatjs/cli is updated to match the peerDependency of typescript so that only a single version is installed in our dependencies.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
The babel presets are no longer requires, since vite/esbuild takes care of that. The module resolution and formatjs plugins are still necessary since the tsconfig.json doesn't cover that. When imports are converted to use the @/ prefix, we can drop the module resolution plugin.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
Now that we're switching to TS, we can fully rely on the Vite toolchain which no longer requires most of the babel plugins to deal with JSX and React.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
Added some missing type definitions, but not everything is there yet.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
We get some conflicts from leaflet libraries and some @storybook/test shenanigans, additionally there are some errors from react-use. It's not feasible to address these right now since we're aiming to get a foot in the door and get started with typing our own code. The offending files: Found 12 errors in 8 files. Errors Files 1 node_modules/@open-formulieren/leaflet-tools/lib/rd.d.ts:8 1 node_modules/@vitest/expect/dist/chai.d.cts:16 3 node_modules/react-leaflet-draw/src/index.d.ts:4 3 node_modules/react-leaflet/lib/LayersControl.d.ts:9 1 node_modules/react-leaflet/lib/SVGOverlay.d.ts:11 1 node_modules/react-use/lib/useEnsuredForwardedRef.d.ts:1 1 node_modules/react-use/lib/usePermission.d.ts:10 1 node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts:16 Hopefully in the future we can re-enable this again.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
Added some missing type definitions, but not everything is there yet.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
We get some conflicts from leaflet libraries and some @storybook/test shenanigans, additionally there are some errors from react-use. It's not feasible to address these right now since we're aiming to get a foot in the door and get started with typing our own code. The offending files: Found 12 errors in 8 files. Errors Files 1 node_modules/@open-formulieren/leaflet-tools/lib/rd.d.ts:8 1 node_modules/@vitest/expect/dist/chai.d.cts:16 3 node_modules/react-leaflet-draw/src/index.d.ts:4 3 node_modules/react-leaflet/lib/LayersControl.d.ts:9 1 node_modules/react-leaflet/lib/SVGOverlay.d.ts:11 1 node_modules/react-use/lib/useEnsuredForwardedRef.d.ts:1 1 node_modules/react-use/lib/usePermission.d.ts:10 1 node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts:16 Hopefully in the future we can re-enable this again.
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 3, 2025
Turns out we need the babel presets after all for the storybook processing :( I'm not sure why things work without in the renderer repo, but we'll figure it out at some point. This strongly types the Body component with its 'component' prop, allowing strings of intrinsic elements to be passed. Support for custom react components is dropped, as it doesn't appear to be used anywhere so this simplifies things a bit.
sergei-maertens
added a commit
that referenced
this issue
Mar 4, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 4, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 4, 2025
* Added a test id for testing-library to the map container so that the interaction tests only start running when the container is visible * Added robustness for interaction elements that may exist in the DOM but not be visible yet because map tiles are still loading waitFor has longer default timeouts than canvas.findBy* queries, and while the latter can be specified, the element is find before it's visible and there are still race conditions. For this reason, the visibility check is decoupled from the lookup expectation.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
Bumped from TS 4.9 to 5.7. TS 5.8 released a couple of days ago and is not yet properly supported (via peerDependencies) in our other tooling, so we hold off on that for a while. @formatjs/cli is updated to match the peerDependency of typescript so that only a single version is installed in our dependencies.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
The babel presets are no longer requires, since vite/esbuild takes care of that. The module resolution and formatjs plugins are still necessary since the tsconfig.json doesn't cover that. When imports are converted to use the @/ prefix, we can drop the module resolution plugin.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
Now that we're switching to TS, we can fully rely on the Vite toolchain which no longer requires most of the babel plugins to deal with JSX and React.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
Added some missing type definitions, but not everything is there yet.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
We get some conflicts from leaflet libraries and some @storybook/test shenanigans, additionally there are some errors from react-use. It's not feasible to address these right now since we're aiming to get a foot in the door and get started with typing our own code. The offending files: Found 12 errors in 8 files. Errors Files 1 node_modules/@open-formulieren/leaflet-tools/lib/rd.d.ts:8 1 node_modules/@vitest/expect/dist/chai.d.cts:16 3 node_modules/react-leaflet-draw/src/index.d.ts:4 3 node_modules/react-leaflet/lib/LayersControl.d.ts:9 1 node_modules/react-leaflet/lib/SVGOverlay.d.ts:11 1 node_modules/react-use/lib/useEnsuredForwardedRef.d.ts:1 1 node_modules/react-use/lib/usePermission.d.ts:10 1 node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts:16 Hopefully in the future we can re-enable this again.
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 5, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Doing a click + ArrowDown on a react select seems to have a race condition where sometimes the *next* option gets 'focus', resulting in it being higlighted with a different color, which in turn trips the Chromatic snapshots as it's toggling between which option is focused. Replacing this with an explicit focus + ArrowDown causes the menu to open and seems to avoid the race condition. The click used to be required in older versions of testing library and react-select to properly focus the input/select when the browser window itself doesn't have focus, e.g. when live-reloading saved code in SB which would lead to a frustrating DX.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Adapted the few places where we directly import the mixin for certain components, which has been moved into the package src and is no longer in the dist/css directory. Other imports have been updated to refer to the package itself, as that resolves to the CSS file inside, and we can now use the @use rule in a bunch of places rather than sticking with the deprecated @import.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Contains fix for the breaking alert component changes.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Contains fix for the breaking alert component changes.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
The real test to see if we have broken CSS 😬
10 tasks
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
The latter is typescript compatible.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
And immediately there is a type issue where strings are passed down to FormattedNumber, which is invalid.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Explicit @/ imports are preferred as they deliberately signal that it's code from our own root/src dir. This will eventually allow us to clean up the rest of the import sorting config.
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
Type checker reported bugs: * subtracting dates from each other (sort-of false positive...) * the way we handle params/headers in API calls didn't account for null * some parseInt call had the radix _outside_ of the function call
sergei-maertens
added a commit
that referenced
this issue
Mar 6, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
And immediately there is a type issue where strings are passed down to FormattedNumber, which is invalid.
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
Explicit @/ imports are preferred as they deliberately signal that it's code from our own root/src dir. This will eventually allow us to clean up the rest of the import sorting config.
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
Type checker reported bugs: * subtracting dates from each other (sort-of false positive...) * the way we handle params/headers in API calls didn't account for null * some parseInt call had the radix _outside_ of the function call
sergei-maertens
added a commit
that referenced
this issue
Mar 10, 2025
Dropped the value prop in favour of always passing children.
sergei-maertens
added a commit
that referenced
this issue
Mar 11, 2025
Dropped the value prop in favour of always passing children.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Check if we can start gradual typescript adoption, normally CRA should support it out of the box?
Steps:
The text was updated successfully, but these errors were encountered: