Skip to content
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

Vite: Remove module-graph scanner #16631

Merged
merged 18 commits into from
Feb 20, 2025
Merged

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Feb 18, 2025

Alternative to #16425

Fixes #16585
Fixes #16389
Fixes #16252
Fixes #15794
Fixes #16646
Fixes #16358

This PR changes the Vite plugin to use the file-system to discover potential class names instead of relying on the module-graph. This comes after a lot of testing and various issue reports where builds that span different Vite instances were missing class names.

Because we now scan for candidates using the file-system, we can also remove a lot of the bookkeeping necessary to make production builds and development builds work as we no longer have to change the resulting stylesheet based on the transform callbacks of other files that might happen later.

This change comes at a small performance penalty that is noticeable especially on very large projects with many files to scan. However, we offset that change by fixing an issue that I found in the current Vite integration that did a needless rebuild of the whole Tailwind root whenever any source file changed. Because of how impactful this change is, I expect many normal to medium sized projects to actually see a performance improvement after these changes. Furthermore we do plan to continue to use the module-graph to further improve the performance in dev mode.

Test plan

  • Added new integration tests with cases found across the issues above.
  • Manual testing by adding a local version of the Vite plugin to repos from the issue list above and the tailwindcss playgrounds.

@philipp-spiess philipp-spiess force-pushed the feat/vite-rm-module-graph-scanner branch from 184bf2c to e47ec4b Compare February 18, 2025 13:46
Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It terrifies me how much code you were able to remove 🙈

Love all the additional integration tests!

Comment on lines 229 to 230
this.buildDependencies.clear()
this.buildDependencies = new Map()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for both a .clear() and = new Map()? Are we using the map as a key in another map somewhere? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no this is just a leftover I think 👍

@philipp-spiess philipp-spiess merged commit 88b762b into main Feb 20, 2025
5 checks passed
@philipp-spiess philipp-spiess deleted the feat/vite-rm-module-graph-scanner branch February 20, 2025 14:23
philipp-spiess added a commit that referenced this pull request Feb 20, 2025
The Nuxt preview server always starts on port 3000 even if that port is
taken. With the added tests in #16631 there is now a higher chance these
ports are already taken since e.g. react router prefers to start at port
3000 and so do other servers.
This PR changes this so that we assign a random port inside the test
instead.

## Test plan

- Ensure Windows CI is green again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment