fix: replace version in generateBundle #12700
Conversation
🦋 Changeset detectedLatest commit: afae019 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| SUBSTITUTION_APP_VERSION_HASH, | ||
| version_hash.padEnd(SUBSTITUTION_APP_VERSION_HASH.length, ' ') | ||
| ) | ||
| .replaceAll(SUBSTITUTION_APP_VERSION, JSON.stringify(kit.version.name).slice(1, -1)); |
There was a problem hiding this comment.
Any danger of this messing with source map positions?
There was a problem hiding this comment.
There is, tried to mitigate that for the hash with the padding but doing so for the version would change the version string.
Could also match the surrounding quotes and add the padding after those.
There was a problem hiding this comment.
Will padding suffice? i.e. will the replacement always be smaller than the placeholder?
(I think matching the surrounding quotes and add padding after is a good idea)
There was a problem hiding this comment.
Fixed now I think,
if version is longer than placeholder then the placeholder is now padded with underscores
if version is shorter than placeholder then version is padded with spaces after the quotes
packages/kit/src/constants.js
Outdated
| @@ -9,3 +9,6 @@ export const GENERATED_COMMENT = '// this file is generated — do not edit it\n | |||
| export const ENDPOINT_METHODS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'HEAD']; | |||
|
|
|||
| export const PAGE_METHODS = ['GET', 'POST', 'HEAD']; | |||
|
|
|||
| export const SUBSTITUTION_APP_VERSION_HASH = '__SVELTEKIT_APP_VERSION_HASH__'; | |||
There was a problem hiding this comment.
These two constants could use a jsdoc comment explaining a bit why we need them (basically a trimmed down version of the PR description)
There was a problem hiding this comment.
addressed in 79836ef (#12700)
|
Dunno why the type check is failing, in my editor it narrows down the type to OutputChunk but the cli doesn't. |
It happens for me too. Must be Kit's version of TypeScript. If you open the package as root in Code and use the workspace TypeScript version (5.4.5) you'll see the error in the IDE. We should probably bump the dev typescript version to 5.5+
|
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
|
I believe this pr is introducing a pretty critical bug when serving the application behind cdn, documented in #12771 |
* Revert "fix: replace version in generateBundle (#12700)" This reverts commit 3591411. #12700 did introduce a bug where the file name stays the same, but the contents of a file can change because of a changed version. Revert for now and take another look at tackling this later. Fixes #12771 Reopens #12260 * changeset
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/environmentfor 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 codeIn 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
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits