Skip to content

fix: replace version in generateBundle #12700

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Oct 7, 2024
Merged

fix: replace version in generateBundle #12700

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1355af3
replace version in generateBundle
gtm-nayan Sep 21, 2024
5d33d85
changeset
gtm-nayan Sep 21, 2024
474f425
format
gtm-nayan Sep 21, 2024
aa5ad52
whoops
gtm-nayan Sep 21, 2024
79836ef
address review comments
gtm-nayan Sep 21, 2024
5345836
mitigate sourcemap damage
gtm-nayan Sep 21, 2024
fbc5cd4
format
gtm-nayan Sep 21, 2024
1bc9d80
missed one
gtm-nayan Sep 22, 2024
7815107
workaround ts type narrowing failure
gtm-nayan Sep 22, 2024
fb26894
Update .changeset/moody-zoos-compare.md
gtm-nayan Oct 7, 2024
afae019
Update .changeset/moody-zoos-compare.md
dummdidumm Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/moody-zoos-compare.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: replace version in generateBundle
3 changes: 3 additions & 0 deletions packages/kit/src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__';
Copy link
Member

Choose a reason for hiding this comment

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

These two constants could use a jsdoc comment explaining a bit why we need them (basically a trimmed down version of the PR description)

Copy link
Member

Choose a reason for hiding this comment

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

addressed in 79836ef (#12700)

export const SUBSTITUTION_APP_VERSION = '__SVELTEKIT_APP_VERSION__';
46 changes: 41 additions & 5 deletions packages/kit/src/exports/vite/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
sveltekit_server
} from './module_ids.js';
import { resolve_peer_dependency } from '../../utils/import.js';
import { SUBSTITUTION_APP_VERSION, SUBSTITUTION_APP_VERSION_HASH } from '../../constants.js';

const cwd = process.cwd();

Expand Down Expand Up @@ -384,7 +385,7 @@ async function kit({ svelte_config }) {
const browser = !options?.ssr;

const global = is_build
? `globalThis.__sveltekit_${version_hash}`
? `globalThis.__sveltekit_${browser ? SUBSTITUTION_APP_VERSION_HASH : version_hash}`
: 'globalThis.__sveltekit_dev';

if (options?.ssr === false && process.env.TEST !== 'true') {
Expand Down Expand Up @@ -470,10 +471,8 @@ async function kit({ svelte_config }) {
}

case sveltekit_environment: {
const { version } = svelte_config.kit;

return dedent`
export const version = ${s(version.name)};
export const version = ${s(is_build ? SUBSTITUTION_APP_VERSION : kit.version.name)};
export let building = false;
export let prerendering = false;

Expand Down Expand Up @@ -923,7 +922,44 @@ async function kit({ svelte_config }) {
}
};

return [plugin_setup, plugin_virtual_modules, plugin_guard, plugin_compile];
/** @type {import('vite').Plugin} */
const plugin_replace_version_and_hash = {
name: 'vite-plugin-svelte-replace-version-and-hash',
enforce: 'post',

generateBundle(_, bundle, __) {
if (vite_config.build.ssr) return;

for (const file in bundle) {
if (bundle[file].type !== 'chunk') continue;
if (
!(
bundle[file].code.includes(SUBSTITUTION_APP_VERSION) ||
bundle[file].code.includes(SUBSTITUTION_APP_VERSION_HASH)
)
)
continue;

// replace the version after the chunk hash has already been calculated
const code = bundle[file].code
.replaceAll(
SUBSTITUTION_APP_VERSION_HASH,
version_hash.padEnd(SUBSTITUTION_APP_VERSION_HASH.length, ' ')
)
.replaceAll(SUBSTITUTION_APP_VERSION, JSON.stringify(kit.version.name).slice(1, -1));
Copy link
Member

Choose a reason for hiding this comment

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

Any danger of this messing with source map positions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


bundle[file].code = code;
}
}
};

return [
plugin_setup,
plugin_virtual_modules,
plugin_guard,
plugin_compile,
plugin_replace_version_and_hash
];
}

/**
Expand Down
Loading