-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat!: Webpack/Jest to Vite/Vitest migration #2716
Conversation
- react/react-dom and other react-related depdencies updated - @testing-library/react-hooks removed as it is now merged into @testing-library/react - focus-trap-react updated to 9.0.2 (upgrading beyond this version will break IE11 compatibility) - tabbable.js mock file added to fix JSDom testing compatibility issues with Modal (with a comment added to Modal test file) - DatePicker tests updated to wait on the next expect after async actions such as open/close/unmount - Various type fixes including... - SocialLink logic tweaked to fix type issues - Prop typing on InPageNavigation updated to omit the content property from the joined JSX.IntrinsicElements['div'] type as it is being overidden (not combined) to prevent type issues
- match original package output - add all other possible bundle formats
- jest and related jest dependencies and config removed - vitest and plugins added - styleMock no longer needed (vitest has css option) - other mocks updated to ESM syntax - project custom types updated - snapshots updated (comments and label format differences) - tests updated to reference vi instead of jest - tests' usage of mocking updated (async with helper for importActual) - tests' usage of creating mocked vars updated (vi has helper function `mocked` for typescript typing) - bad path for a uswds svg fixed - other minor fixes due to jest/vitest variances
- yarn.lock recreated from scratch - type fixes - FocusTrap component mocked (fix JSDom incompatibility) - test fixes - linting fixes - test coverage options added
- sb init ran which added some new defaults such as... - addons: interactions, links, onboarding, blocks, react-vite, test - eslint plugin with extension of recommended rules - sb config replacement for vite
- babel dependencies removed - sb onboarding addon removed - focus-trap-react updated to latest - happo dependencies updated to latest - all sb dependencies on latest version - vite svgr config fixed to target `*.svg?svgr` urls - sb `custom-styles.scss` updated to use project's `styles/_uswds-theme.scss` file for settings - sb `public` folder added to refreshed sb config
- remove sb dependencies from dependabot ignore - remove node 16 from build-and-test matrix - package.json engine updated to node >= 18
As mentioned, this is merely a proof-of-concept to show one direction that the project can go to not use webpack. It is intended foremost to be educational and provide insight. There are no expectations that this goes beyond draft, but if it is desired then necessary cleanup will happen (ex: vite config for various builds). Changes to conform to vite project norms were kept within the current file structure (project typescript type file, tsconfig, etc.). With maintainer blessing those file norms can be applied. Vite uses a mix of esbuild and rollup under the hood. The lib mode has it's fair share of warts to work around like any bundler but I think I have it tamed. I have it creating bundles for every format it supports, because why not XD . Vite/Vitest are ES-first, thus the move from Jest (ES support on Jest is still experimental). With jest migrated to vitest the configuration needed to run vite, vitest, and storybook is now capable of being handled under one roof. There are some important differences in how jest/vitest work (especially mocks) that need to be considered. With webpack/jest usage removed, I was able to drop all of their directly-related dependencies along with babel ones. The version of JSDom being used apparently requires node 18+ so there were project changes in that regard (drop node 16 related stuff). USWDS since 3.0 has not supported IE so the IE11 line in the browser support list was dropped and focus-trap-react updated to current major version. The JSDom workaround for focus-trap-react uses a mock for the react library that enforces the necessary setting to always be disabled instead of the recommended mock for its dependency as I currently could not get vitest to handle loading it directly for mocking (aka, it currently is not dev-friendly to tell Vitest to handle loading dependencies-of-dependencies. direct dependencies that you import in your code mocks fine). There are warnings that appear now during linting, mostly from the new storybook plugin. Getting the eslint config right to handle cli-side and lib-side stuff properly is where i'm the weakest. The yarn lock file was recreated from scratch and fixes made from errors that cropped up. Updating the version of yarn used in this project is very recommended (and would allow removing one of the dependencies being manually resolved to a specific version). Vite feature link: https://vitejs.dev/guide/features.html |
Tagging current maintainer on rotation: @werdnanoslen |
Additional work to do a dependency-roundup of sorts to get all dependencies updated (including the React 18 upgrade) without errors can be done if moving this beyond draft is desired (as this PR would be a good time to do it due to its slight disruptiveness). |
- precommit checks for lint, format:fix, and test (changed only)
- pascalCase for story names - identical test names in same block - expect methods not being properly called
- fix FooterExtendedNavList tests to render with css definition of "hidden" class in order to assert visibility on elements - missing testid added to masthead test - filepreview skipped test area for generic previews updated to test across all types/extensions scenarios properly and no longer skipped
- use `--mode uswds` to build uswds
- control firing of loadend event for filepreview test to prevent race condition
@jpandersen87 we'd like to test this out on some internal projects soon — if this is ready, can you resolve the conflict please? |
@werdnanoslen Branch updated |
@jpandersen87 thanks so much for bearing with us while we found opportunities on projects to test this out! We'd be happy to approve and merge this as soon as those remaining conflicts are resolved. 🚀 |
I take that as meaning tests have gone well? Branch has been updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't need my approval, I don't think, but putting the stamp here since I tested it with success on my current project.
Just disabled the Node16 tests branch protection rule, since this PR removes tests for that legacy node version as well (rightly so). Once the pipeline re-runs, I expect it to be all green this time! 🤞🏻 |
In v8.0.0, I'm getting an error when I try to import
and the import in import '@trussworks/react-uswds/lib/index.css'; UPDATE: PR created: #2807 |
Summary
This is a conversion from using webpack/jest as bundler/test library to vite/vitest. This will ensure the react-uswds library can support the new ESM standard, while maintaining backwards compatibility with CJS. All npm scripts have been updated accordingly to perform the same intended actions. Building is two steps: build the library and build the uswds assets (css, related files, etc.) dictated by the
--mode uswds
argument (running justbuild
will do these steps for you).Vite
Vitest
Also included:
Notes:
.happo.cjs
filebundles.test.tsx
file was added that is excluded during normal testing and is ran after building to do basic importing and rendering tests on each bundle formatRelated Issues or PRs
#192
How To Test