Conversation
ty2k
left a comment
There was a problem hiding this comment.
Let's split this up into even smaller PRs to de-risk this:
- One for just the React Aria Components version bump
- This includes the additional new dependencies for
DatePickerto work in v1.17.0, but these need to go intodependencies, see notes below.
- This includes the additional new dependencies for
- One for just the ESLint changes
- ESLint v9 -> v10
- New addition of
@eslint/js
- One for just the TypeScript major version bump without any configuration changes
- Optionally, after everything else is working cleanly, one for any TypeScript configuration changes. IMO this is lowest priority and optional, noting that we've had the
allowImportingTsExtensionswarning going back at least months, like in the job logs here.
| @@ -0,0 +1 @@ | |||
| declare module "*.css"; | |||
There was a problem hiding this comment.
It doesn't seem like this change or the rest of the changes in 4137ee9 are required for this to go. Can we please take it out and worry about it in a separate PR of just TypeScript fiddling to de-risk this?
| typescript({ | ||
| tsconfig: "./tsconfig.build.json", | ||
| include: ["src/index.ts", "src/components/**/*"], | ||
| include: ["src/index.ts", "src/components/**/*", "src/**/*.d.ts"], |
| /* Bundler mode */ | ||
| "moduleResolution": "bundler", | ||
| "allowImportingTsExtensions": true, | ||
| "rewriteRelativeImportExtensions": true, |
| "dependencies": { | ||
| "@bcgov/design-tokens": "3.2.0", | ||
| "react-aria-components": "1.16.0" | ||
| "react-aria-components": "1.17.0" |
There was a problem hiding this comment.
Let's move this change to another PR to de-risk.
| "@react-stately/data": "3.15.0", | ||
| "@rollup/plugin-commonjs": "29.0.0", | ||
| "@react-aria/datepicker": "3.17.0", | ||
| "@react-aria/i18n": "3.13.0", |
There was a problem hiding this comment.
These two new @react-aria/* dependencies need to go into dependencies instead of devDependencies because we are using useDisplayNames and useDateFormatter right in DatePicker, so this code needs to ship with the component library (they are direct dependencies) as opposed to only being used for development tasks (devDependencies). Contrast that with something like @react-stately/data, where we're only using in the Vite kitchen sink app, not in the actual component.
It looks like the need for this might be the result of a change to how dependencies work in react-aria-components v1.17.0: https://react-aria.adobe.com/releases/v1-17-0
This PR updates:
typescriptfrom 5.9.3 -> 6.0.3eslintfrom 9.39.2 -> 10.2.0react-aria-componentsfrom 1.16.0 -> 1.17.0These changes are cherry-picked from #655 (comment).
Additional changes in this PR:
tsconfig.json(see 6.0 Migration Guide microsoft/TypeScript#62508 (comment)), to resolve a console warningrootDirproperty totsconfig.types.json(see https://github.com/6.0 Migration Guide microsoft/TypeScript#62508#issuecomment-3348659946), to resolve a build errortsconfig.jsonandrollup.config.js, and adds a new declarations file to resolve new TypeScript errors (TS5096 and TS2282) that popped up on buildAs far as I can tell there are now no outstanding issues:
✅ test script runs successfully
✅ lint script runs successfuly
✅ build script runs successfully
✅ Storybook builds successfully
✅ Vite app builds successfully