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: do not append browserHash on optimized deps during build #13906

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

lsdsjy
Copy link
Contributor

@lsdsjy lsdsjy commented Jul 20, 2023

Description

Do not append browserHash on optmized deps during build. It is unnecessary and on some occasions would cause duplicate files bundled. For example, if we have multiple entries, vite will "discover and optimize new deps" and re-compute the browserHash, so it could end up with both .vite/deps_build-dist/chunk-ABCDEFGH.js?v=11111111 and .vite/deps_build-dist/chunk-ABCDEFGH.js?v=22222222 in the final bundles. They have the same content but different ids, so rollup treats them as different modules.

In fact, "entry" optmized deps have no browserHash suffix currently. Only chunks imported by "entry" optimized deps have that. For example, .vite/deps_build-dist/vue-demi.js imports vite/deps_build-dist/chunk-ABCDEFGH.js?v=11111111 and .vite/deps_build-dist/vue.js imports .vite/deps_build-dist/chunk-ABCDEFGH.js?v=22222222.

I think this happens when the user cannot correctly configure the entries for deps optimization, which is possible for complicated and highly customized projects.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@stackblitz
Copy link

stackblitz bot commented Jul 20, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This sounds good to me, but the issue that prompted you to create this fix does not. Even if you have multiple entries, there should be a single build process and all deps must be found before optimizing. There shouldn't be any new deps discovered during build. Would you create a minimal reproduction and a bug report?

@lsdsjy
Copy link
Contributor Author

lsdsjy commented Jul 20, 2023

You're right but I'm having some trouble with creating a minimal reproduction. I'll file an issue if I'd make it!

@lsdsjy lsdsjy changed the title fix: do not append browserHash on optmized deps during build fix: do not append browserHash on optimized deps during build Jul 20, 2023
@patak-dev patak-dev merged commit 0fb2340 into vitejs:main Jul 21, 2023
@lsdsjy
Copy link
Contributor Author

lsdsjy commented Jul 21, 2023

I found that it cannot be reproduced on newer vite versions. The last version where I can reproduce the "chunks with different hash" behavior is 4.2.3 and the first version that cannot reproduce is 4.3.3. However I cannot find the specific version that fixes this because my project cannot build on vite >= 4.3.0 <= 4.3.2 (it throws error in vite:optimized-deps-build). I guess it's related to some commits about deps optimizer in 4.3.0.

@maxpatiiuk
Copy link

maxpatiiuk commented Sep 16, 2024

I am having a bug in Vite 5.4.5, possibly related to this Pull request, where dependency is loaded in the browser twice - once with browser hash, and once without:

Screenshot 2024-09-16 at 15 20 05

I am trying to create a minimal reproducible repo and will open a ticket.

Screenshot 2024-09-16 at 15 20 24

Screenshot 2024-09-16 at 15 20 21

@maxpatiiuk
Copy link

Found out the cause:

I was calling server.listen() and server.transformIndexHtml() in parallel. I didn't realize they would be interfering and creating a race condition.

server.listen() starts up the dependency pre-bundler, which makes vite:resolve plugin add ?v=... to identifiers.
The identifiers that were resolved in server.transformIndexHtml() before dependency pre-bundler started won't have ?v=..., but the identifiers resolved a bit later will. This race condition introduces a chance that the same module will be resolved once with and once without ?v=....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants