-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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(optimizer): check optimizeDeps.extensions for scannable files #14543
feat(optimizer): check optimizeDeps.extensions for scannable files #14543
Conversation
Actually hold on a second my logic isn't correct in the |
089b06e
to
f571795
Compare
@bluwy it should be good now. Let me know if you have any feedback on this implementation! |
f571795
to
bec9eb2
Compare
@bluwy I went ahead and reverted to my original implementation (with |
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.
Awesome. Thanks! Will wait for one more set of eyes for this since it's expanding an existing feature.
@bluwy is there a timeline for the Vite 5 release? If it's expected to be a while it would be nice for us if this could be backported. 🙏 |
The Vite 5 stable release is planned mid October, though with the progress so far it could be a little late, but shouldn't extend till next month. I don't think this fix could be backported though, it's leaning towards a bigger change that could affect downstream (actually let me run this through ecosystem-ci). If we would to backport something, I think it would be #14536 instead. |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
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.
This makes sense to me, and it kind of feels more like a fix than a new feature. I think we can merge it.
Description
Updates the
isScannable
logic to include extensions registered viaoptimizeDeps.extensions
.This allows plugins (such as
@marko/vite
) to contribute to the optimized dependency scanning if they supply an esbuild plugin.Additional context
Currently due to the way
@marko/vite
is setup it relied exclusively onoptimizeDeps.include
and not the dependency scanning so adding this change does break existing Marko apps.However @marko/vite is being updated to use
optimizeDeps.entries
instead since the include method we were using needlessly bundles more than necessary.The actual processing of Marko files is done by an esbuild hook added by
@marko/vite
so no further change is needed here other than allowing.marko
files to pass through theisScannable
checks.This issue was addressed in a different way in #14536 and so if this solution is deemed better we can close that PR.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).