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

fix: respect optimize deps entries #8489

Merged
merged 3 commits into from
Jun 8, 2022
Merged

Conversation

patak-dev
Copy link
Member

Description

Once we merged:

We stopped respecting optimizeDeps.entries. This PR uses this config option to let users be explicit about which extra entries should be processed before the first deps optimization step.

Right now, the optimization is run after every static import (+ dynamic imports that are triggered by the browser while loading the first visited page) are processed. This seems like a good default because processing every dynamic import may be more costly now that we don't do it with the esbuild scanner, especially for apps like StoryBook-like apps.

After this PR, the user should add to entries every dynamic import entry that is commonly used after the first load.

TODO: Tests.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev added the p3-significant High priority enhancement (priority) label Jun 7, 2022
@patak-dev patak-dev requested a review from antfu June 7, 2022 14:07
@patak-dev
Copy link
Member Author

The PR now includes a refactor to move the delay until idle logic from the optimizedDeps plugin inside DepsOptimizer (another step in having a single place to control this logic). We need this now because we should process the optimizeDeps.entries only on cold start.

With the logic in DepsOptimizer, we could implement in another PR that we always start a new delay until idle instead of only doing it the first time and then falling back to the old debounce. Every time there is a new dynamic import, it would be better to until all new sources have been processed.

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nitpick, but great refactor overall!

packages/vite/src/node/optimizer/index.ts Show resolved Hide resolved
packages/vite/src/node/optimizer/optimizer.ts Outdated Show resolved Hide resolved
@patak-dev patak-dev merged commit fba82d0 into main Jun 8, 2022
@patak-dev patak-dev deleted the fix/respect-optimize-deps-entries branch June 8, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-significant High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants