Skip to content

fix: do not append browserHash on optimized deps during build#13906

Merged
patak-cat merged 2 commits intovitejs:mainfrom
lsdsjy:patch-2
Jul 21, 2023
Merged

fix: do not append browserHash on optimized deps during build#13906
patak-cat merged 2 commits intovitejs:mainfrom
lsdsjy:patch-2

Conversation

@lsdsjy
Copy link
Contributor

@lsdsjy lsdsjy commented Jul 20, 2023 •
edited
Loading

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.

@bolt-new-by-stackblitz
Copy link

bolt-new-by-stackblitz bot commented Jul 20, 2023

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

patak-cat
patak-cat approved these changes Jul 20, 2023
View reviewed changes
Copy link
Member

@patak-cat patak-cat 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-cat patak-cat 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.

@lsdsjy lsdsjy mentioned this pull request Dec 5, 2023
Closed
7 tasks
@maxpatiiuk
Copy link
Contributor

maxpatiiuk commented Sep 16, 2024 •
edited
Loading

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
Contributor

maxpatiiuk commented Sep 17, 2024

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

Reviewers

@patak-cat patak-cat patak-cat approved these changes

@bluwy bluwy bluwy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lsdsjy @maxpatiiuk @patak-cat @bluwy