-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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: optimize deps option to turn off auto discovery #13000
Conversation
Run & review this pull request in StackBlitz Codeflow. |
LGTM! 👍 I wouldn't worry about the default true/false much, as long as we provide jsdocs and documentation. I understand that making options opt-in would be a good design pattern for understanding, but I feel that only work if all the options are falsy default (that might need to whole set of breaking changes) |
I think the same @antfu. In the end, I switched this one to |
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.
I like this 👍🏻 Thank you 🙏🏻
Vitest probably cannot remove inline
for some time now, but if this feature will be more performant then we can recommend it over inline
👍🏻
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.
Nice tests!
Starting discussion to stabilize this feature: |
Description
Edit: I ended up renaming it to
noDiscovery
.Adds a new experimental boolean option
optimizeDeps.noDiscovery
, defaulting totrue
@sheremet-va is exploring replacing Vitest
deps.inline
feature with Vite's deps optimization. To make this work, they need auto-discovery of dependencies to be turned off. They would like to have full control of what dependencies get optimized usingoptimizeDeps.include
. We should also merge #12414 to add glob support tooptimizeDeps.include
Some other options we evaluated in this gist.
Edit: scratch the next paragraph, as it simplified the logic to rename at the end.
@bluwy also proposed we use
noDiscovery
instead ofautoDiscovery
so we can have the option be false by default. I think this is a good idea in general but in this case, I find it a bit more confusing. @bluwy I checked and we have a lot of boolean options that default to true.preTransformRequests
for example. Let's see what others think.It is experimental because @sheremet-va will need to play with this and we may end up changing this feature later. I think we could move forward so they can check what we are missing.
What is the purpose of this pull request?