-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
perf(experimental): add file system cache #9026
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
Conversation
@vitest/browser
@vitest/browser-playwright
@vitest/browser-preview
@vitest/browser-webdriverio
@vitest/coverage-istanbul
@vitest/coverage-v8
@vitest/expect
@vitest/mocker
@vitest/pretty-format
@vitest/runner
@vitest/snapshot
@vitest/spy
@vitest/ui
@vitest/utils
vitest
@vitest/web-worker
@vitest/ws-client
commit: |
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Not sure how we can get rid of the stale cache without reducing the performance 🤔 |
|
Forgot to mention that I have a concern about transform cache when plugin collects module information through One example which I thought might hit is tailwind, but now looking at it, it might have switched to its own fs crawling entirely. |
hi-ogawa
left a comment
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.
Have you considered nuking cache based on lockfile like Vite's deps optimizer? That can at least cover packaged plugin change.
That could work, yeah. |
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.
The update looks good.
I quickly reviewed through Vite's builtin plugins and one feature which can get broken by the cache is import.meta.glob like transform (e.g. import.meta.glob("./*"), import(`./{var}`) (dynamic import var), new URL(`./{var}`, import.meta.url) (asset/worker import url)).
Another type of breakage is, user's tsconfig can affecting builtin esbuild/oxc transform and also 3rd party tsconfig plugin with paths alias. I think this is "less surprising" breakage since it's an obvious config change. Similar can happen when simply tweaking option for any plugin until defineCacheKeyGenerator is implemented.
I don't think this is blocking the experimental feature though. The question is, should we handle known patterns (at least for Vite builtin plugin) to bail out caching or do we add defineCacheKeyGenerator to upstream?
One more thing I just remembered is Rollup has shouldTransformCachedModule hook to let plugin decide cache https://rollupjs.org/plugin-development/#shouldtransformcachedmodule. I don't think Vite supports this and I don't think it's straight forward to actually reuse the same hook signature, but just sharing as a reference. For example, searching this hook https://github.com/search?q=shouldTransformCachedModule&type=code might show plugins which can break with fs cache.
This is now addressed in the latest commit (only defineCacheKeyGenerator(({ sourceCode }) => {
// assuming every new URL is dynamic
if (sourceCode.includes('new URL')) {
return false // false means don't cache this
}
})Dynamic imports and assets are also only affected in the
This should not be a problem because we include the content of the config file and its dependencies inside the hash. Adding a few more tests and it should be good to go. |
Description
This PR adds a file system module cache that can be enabled via
experimental.fsModuleCache.Having a file system cache makes it faster to rerun tests on big projects.
Fixes #6441
Some numbers with FS Cache (4.0.11):
first run
vs second run