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: 🧪 chunkMap #15373

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: 🧪 chunkMap #15373

wants to merge 2 commits into from

Conversation

bhbs
Copy link
Contributor

@bhbs bhbs commented Dec 17, 2023

for Production: #16552


Proof of Concept Implementation for #15372

This PR shows that ChunkMap (to avoid cascading cache invalidation problem with importmap) can be implemented seamlessly without impact on existing systems.

Copy link

stackblitz bot commented Dec 17, 2023

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

@bhbs bhbs force-pushed the chunkMap branch 2 times, most recently from 20dfec4 to 46a8ae1 Compare December 18, 2023 10:27
@bhbs bhbs changed the title feat: chunkMap feat: 🧪 chunkMap Dec 18, 2023
@bhbs bhbs force-pushed the chunkMap branch 3 times, most recently from a8bc4cd to 392f959 Compare December 19, 2023 15:40
@bluwy bluwy added the p3-significant High priority enhancement (priority) label Dec 22, 2023
@bluwy bluwy mentioned this pull request Jan 13, 2024
6 tasks
@benmccann
Copy link
Collaborator

I wanted to surface a couple of comments about this PR over in the SvelteKit repo that reviewers should probably be aware of: sveltejs/kit#11615 (comment)

@bhbs bhbs force-pushed the chunkMap branch 3 times, most recently from 2a3ff35 to 72e913b Compare January 18, 2024 00:49
@jacekkarczmarczyk
Copy link

Thanks for working on this issue, I've tried it on my actual app and:

  • it seems to work at first glance, two consecutive builds resulted only in changed index-*.js (I'm using build time env var which apparently is not in its own module, so it lands in the main entry point, and only that entry point was changed), then I've changed some .vue file and the result was ok as well

  • I get some error that seems to be related to PWA:

image

Not sure if that's the problem that needs to be fixed in vite or in the PWA plugin

@bhbs
Copy link
Contributor Author

bhbs commented Jan 31, 2024

Thank you for checking ✨
The error doesn't seem to be related to the hash. I wonder what it is.
In systems that, like VitePress, directly load and process generated JS instead of HTML, there can be issues if the import-map is not considered during loading.

@patak-dev
Copy link
Member

Thanks for testing @jacekkarczmarczyk! cc @userquin in case this rings a bell

@jacekkarczmarczyk
Copy link

jacekkarczmarczyk commented Jan 31, 2024

The error doesn't seem to be related to the hash. I wonder what it is.

I've build first with [email protected] and then with this PR, there's a lot of commits since 5.0.5 on the main branch, so error might be caused by one of them and not by this PR. Didn't check it against current main branch, maybe I'll do it tomorrow.

@jacekkarczmarczyk
Copy link

I've rebased PR branch against the current main and PWA error dissapeared👍

@jacekkarczmarczyk
Copy link

I've tested it also with legacy plugin (5.3.0), and all chunks have new hashes, I think it's fine for legacy chunks (old browsers don't support import maps I guess, although it could use System.js, not a big issue though) but modern ones should not be changed.

Not sure if I'm going to use legacy plugin after switching to vite, just letting you know.

@bhbs bhbs force-pushed the chunkMap branch 4 times, most recently from 8f3778e to 1616060 Compare March 8, 2024 00:51
@olivierbeaulieu
Copy link

olivierbeaulieu commented Apr 3, 2024

I've experimented with applying this POC to our large vite app, and it works brilliantly!

@bhbs bhbs force-pushed the chunkMap branch 2 times, most recently from 2352c99 to 9d4dc45 Compare April 21, 2024 05:04
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.

None yet

6 participants