Skip to content

fix: browser cache of newly discovered deps#7378

Merged
patak-cat merged 2 commits intomainfrom
fix/deps-long-term-caching
Mar 19, 2022
Merged

fix: browser cache of newly discovered deps#7378
patak-cat merged 2 commits intomainfrom
fix/deps-long-term-caching

Conversation

@patak-cat
Copy link
Member

@patak-cat patak-cat commented Mar 18, 2022

Description

I'm working to see if it is possible to have the scan phase be also non-blocking, and it is showcasing some issues with the current approach we started at #6758

An important observation is that we can only safely use 'max-age=31536000,immutable' for requests when we are using dependencies versioned with browserHash after a cached start, initial scan phase, or after discovery+full-reload. The browserHash that is computed for each newly discovered deps isn't fit for long-term caching with the approach previous to this PR. The issue is hard to manifest but it could happen when:

  1. There is a newly discovered dep_a that gets assigned bh_a
  2. It gets optimized and there aren't altered files (so it keeps bh_a and has a bundle o_a1)
  3. The cache is cleaned with --force on server restart
  4. There is a newly discovered dep_a that gets assigned bh_a
  5. There is another discovered dep_b that gets assigned bh_b
  6. Both gets optimized together (it can happen as there is a debounce)
  7. There are common chunks between them so dep_a ends up with bh_a but with bundle o_a2.
  8. The browser uses the cached version o_a1 because the version is bh_a

a. This PR solves this issue by including a server timestamp in the browserHash of newly discovered deps.

b. It also fixes versioning of chunks and dynamic imports, as they should get a stable hash, and they can be safely cached long term using the main browserHash

c. It also fixes an issue with the computing of alteredFiles that was broken due to paths, so it was always true.

Note: I think we should add a fileToDepInfoMap to speed up searching for depInfo from a file, which we are starting to do quite a bit.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-cat patak-cat requested a review from bluwy March 18, 2022 17:11
@patak-cat patak-cat added the p3-minor-bug An edge case that only affects very specific usage (priority) label Mar 18, 2022
@patak-cat
Copy link
Member Author

patak-cat commented Mar 18, 2022

✅ All green vite-ecosystem-ci run against this PR
https://github.com/vitejs/vite-ecosystem-ci/actions/runs/2005480978

This was referenced Mar 18, 2022
Merged
Closed
@patak-cat patak-cat linked an issue Mar 19, 2022 that may be closed by this pull request
Vite server crashes when starting dev server after optimizing new dependencies [ERR_OUTDATED_OPTIMIZED_DEP] #7369
Closed
7 tasks
@bluwy
Copy link
Member

bluwy commented Mar 19, 2022

Some questions 😬

7. There are common chunks between them so dep_a ends up with bh_a but with bundle o_a2

I'm curious how/why dep_a ends up with bh_a, given IIUC the browserHash is calculated based on the deps found too (in this case a new dep_b which should create a new browserHash). Or maybe that isn't enough, I wonder if we should also calculate and derive via the imports too? (borrowed from alteredFiles)

b. It also fixes versioning of chunks and dynamic imports, as they should get a stable hash, and they can be safely cached long term using the main browserHash

I'm a bit confused of why we need another hash, given chunks are already hashed in their file name. I think I'm missing a piece here.

@patak-cat
Copy link
Member Author

patak-cat commented Mar 19, 2022

Some questions 😬

  1. There are common chunks between them so dep_a ends up with bh_a but with bundle o_a2

I'm curious how/why dep_a ends up with bh_a, given IIUC the browserHash is calculated based on the deps found too (in this case a new dep_b which should create a new browserHash). Or maybe that isn't enough, I wonder if we should also calculate and derive via the imports too? (borrowed from alteredFiles)

Each discovered dep gets a new browser hash based on every optimized + discovered dep, so deb_b will get a new bh_b has. But dep_a will keep bh_a until we need to do a full-page reload (we can't change the hash of a dep of we will have duplicates). So dep_a hash didn't include dep_b.

b. It also fixes versioning of chunks and dynamic imports, as they should get a stable hash, and they can be safely cached long term using the main browserHash

I'm a bit confused of why we need another hash, given chunks are already hashed in their file name. I think I'm missing a piece here.

Import analysis is also performed in these chunks adding the browser hash to its imported paths, so the hash from the file name isn't enough because for the same file content we may end up needing to use different browser hashes in these imports. See also #7350, this was the problem in #7323

bluwy
bluwy approved these changes Mar 19, 2022
View reviewed changes
Copy link
Member

@bluwy bluwy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

After re-reading the code a few times, I think I understand most of the whys here now. LGTM.

@patak-cat patak-cat merged commit 392a0de into main Mar 19, 2022
@patak-cat patak-cat deleted the fix/deps-long-term-caching branch March 19, 2022 07:47
@patak-cat
Copy link
Member Author

patak-cat commented Mar 19, 2022

Released in vite@2.9.0-beta.4

@infiton infiton mentioned this pull request Aug 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluwy bluwy bluwy approved these changes

Assignees

No one assigned

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Vite server crashes when starting dev server after optimizing new dependencies [ERR_OUTDATED_OPTIMIZED_DEP]

2 participants

@patak-cat @bluwy