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

feat: non-blocking esbuild optimization at build time #8280

Merged
merged 58 commits into from May 26, 2022

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented May 22, 2022

Description

Alternative to:

If we merge:

At #4921, Evan proposed:

Use esbuild to also pre-bundle dependencies for prod, then alias deps to the pre-bundled chunks during the Rollup build phase. This would

  • allow us to drop @rollup/plugin-commonjs and reduce prod/dev inconsistency due to difference in handling CJS between esbuild & @rollup/plugin-commonjs
  • allow us to cache the pre-bundled deps like the dev deps. Although they would still need to be processed by Rollup, it now needs to process fewer files and no longer need to do CJS transforms (essentially these are done by esbuild instead). Could potentially speed up the build to some extent.

In this PR, we don't perform any scanning during build. We know what dependencies needs interop without waiting for esbuild optimization, so we can let rollup process all user source code (so we have something better than the scan phase because transforms are applied). When rollup is iddle, we perform the optimization step and resolve the loading of optimized deps.

Right now it is using a simple delay on a transform hook, but we can surely do something more robust. I think rollup gives us a way to wait until it is only waiting.

Edit: to disable the deps optimizer during build you can use optimizeDeps.disabled: 'build'.
The disabled experimental option has been extended as optimizeDeps.disabled: boolean | 'dev' | 'build'


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@bluwy
Copy link
Member

bluwy commented May 23, 2022

Yeah this feels a lot better! I think #7568 looks good. There's test failing, but I was waiting for it to be out-of-draft to give my approval 😬

@patak-dev
Copy link
Member Author

There seems to be a race condition in #7568 with the /external test in ssr-vue (that now uses your extensions feat @bluwy). I can't reproduce it in Mac though.

@patak-dev
Copy link
Member Author

fix: optimizeDeps.include support address the issue brought by @bluwy here #8265 (comment), one of the skipped tests has been reverted and is passing now. I'm going to continue working on this PR and leave #8265 as reference to showcase the implementation using proxies. But I believe that the approach here is more sound moving forward.

packages/vite/src/node/build.ts Outdated Show resolved Hide resolved
packages/vite/src/node/config.ts Show resolved Hide resolved
packages/vite/src/node/build.ts Outdated Show resolved Hide resolved
packages/vite/src/node/plugins/resolve.ts Outdated Show resolved Hide resolved
packages/vite/src/node/plugins/resolve.ts Outdated Show resolved Hide resolved
@bluwy
Copy link
Member

bluwy commented May 26, 2022

Should we read the dev cached metadata during build? I initially thought this was going to be a key optimization, but now I think that it may be better to keep both caches completely separated. There may be some deps that are dev-only for example, and it would be hard to identify. I think esbuild is fast enough, we can recreate the cache for dev and build.

Agreed that we should keep it separate, especially when we have process.env.NODE_ENV different for both case.

@bluwy, are some of the issues you listed here affecting this PR: #4921 (comment)

Since we're not apply plugins in prebundling yet, I think most of the points there are not applicable for now.

@patak-dev patak-dev requested review from bluwy and antfu May 26, 2022 12:09
@patak-dev
Copy link
Member Author

I think the PR is in a good state now to be merged. It would be good to release an alpha including it for people to test, and keep working to fix possible regressions in future PRs.

@patak-dev patak-dev merged commit 909cf9c into main May 26, 2022
@patak-dev patak-dev deleted the feat/non-blocking-esbuild-optimization-at-built-time branch May 26, 2022 16:45
@ghost ghost mentioned this pull request Jun 2, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistency between dev & build p3-significant High priority enhancement (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants