-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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: separate framework code from app code #8957
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flummoxed by the test failure. Happens reliably, but not locally. |
This reverts commit a08b5d8.
I added some console.logs to see where the failure might be. Seems like |
This reverts commit ec618f5.
Rich-Harris
commented
Feb 17, 2023
5 tasks
Merged
6 tasks
dummdidumm
pushed a commit
that referenced
this pull request
Oct 7, 2024
In #8957 one of the desired outcomes was that the framework code would have its own chunk with a stable hash for better caching, but that didn't really happen because the client imports something that references version, through __sveltekit/environment for created_updated_store and version hash through __sveltekit/path, which meant that when the developer changes their app code and increments/changes the version it'd also change the above mentioned modules and subsequently most chunks with the framework code In this PR the version is injected in generateBundle once the chunk hashes have already been calculated so that just changing the version doesn't affect the hashes for chunks without any actual code changes fixes #12260
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative to #8803, and closes #8464 albeit via a somewhat larger change.
Essentially this splits out the app code from the framework code, and passes app information to the framework via the
start(...)
call. This gives us an opportunity to set environment variables before any app code runs, but it also has a nice side-effect — the framework code now resides in its own chunk, with a stable hash. This will make cache hits more frequent, since thestart.js
chunk will only be invalidated when there's a new framework version, not when there's a new app version.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.