-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
perf: avoid side effects resolving in dev and in the optimizer/scanner #12789
perf: avoid side effects resolving in dev and in the optimizer/scanner #12789
Conversation
Run & review this pull request in StackBlitz Codeflow. |
Co-authored-by: Bjorn Lu <[email protected]>
|
Perf improvements confirmed 🎉 |
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.
You don't have to make me a co-author, but thanks 🙂 One small nit below and I think this is good!
I also noticed we can push a bit further, for container.resolveId
usecases as well like:
const resolved = alreadyResolved ?? (await this.resolveId(url, !!ssr)) |
(await pluginContainer.resolveId(url, undefined, { ssr }))?.id ?? |
but we can do this in another PR if you prefer.
Oh, good ones! So, should we switch from an option in the resolve plugin to a new option passed to |
@bluwy I tried to use |
Sounds good to me. We could probably revisit them again in the future. |
Description
We already had some
isBuild
checks but they weren't in all the places we resolved side effects. I think we should also avoid resolving side effects during deps optimization (both in the scanner and in esbuild).What is the purpose of this pull request?