From 8548245f15d37ede9406308b7fa7fb14c605fc71 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Thu, 17 Apr 2025 20:22:17 +0800 Subject: [PATCH 1/6] add test --- .../inline-assets/dynamic-import/+page.js | 5 ++++ .../inline-assets/dynamic-import/+page.svelte | 5 ++++ .../inline-assets/dynamic-import/Thing.svelte | 8 ++++++ packages/kit/test/apps/options/test/test.js | 27 ++++++++++++++----- 4 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.js create mode 100644 packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.svelte create mode 100644 packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/Thing.svelte diff --git a/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.js b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.js new file mode 100644 index 000000000000..6b6f8e04547c --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.js @@ -0,0 +1,5 @@ +export async function load() { + return { + Thing: (await import('./Thing.svelte')).default + }; +} diff --git a/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.svelte b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.svelte new file mode 100644 index 000000000000..d2a75179b0b2 --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/Thing.svelte b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/Thing.svelte new file mode 100644 index 000000000000..747598bcede0 --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/Thing.svelte @@ -0,0 +1,8 @@ +

I'm dynamically imported

+ + diff --git a/packages/kit/test/apps/options/test/test.js b/packages/kit/test/apps/options/test/test.js index 3ce6b482e092..82a30b34f159 100644 --- a/packages/kit/test/apps/options/test/test.js +++ b/packages/kit/test/apps/options/test/test.js @@ -107,7 +107,7 @@ test.describe('assets path', () => { await page.goto('/path-base/'); const href = await page.locator('link[rel="icon"]').getAttribute('href'); - const response = await request.get(href); + const response = await request.get(href ?? ''); expect(response.status()).toBe(200); }); }); @@ -133,7 +133,7 @@ test.describe('CSP', () => { test('ensure CSP header in stream response', async ({ page, javaScriptEnabled }) => { if (!javaScriptEnabled) return; const response = await page.goto('/path-base/csp-with-stream'); - expect(response.headers()['content-security-policy']).toMatch( + expect(response?.headers()['content-security-policy']).toMatch( /require-trusted-types-for 'script'/ ); expect(await page.textContent('h2')).toBe('Moo Deng!'); @@ -141,7 +141,7 @@ test.describe('CSP', () => { test("quotes 'script'", async ({ page }) => { const response = await page.goto('/path-base'); - expect(response.headers()['content-security-policy']).toMatch( + expect(response?.headers()['content-security-policy']).toMatch( /require-trusted-types-for 'script'/ ); }); @@ -305,19 +305,32 @@ if (!process.env.DEV) { }); test.describe('inlineStyleThreshold', () => { - test('loads asset', async ({ page }) => { + test('loads assets', async ({ page }) => { let fontLoaded = false; - page.on('response', (response) => { if (response.url().endsWith('.woff2') || response.url().endsWith('.woff')) { fontLoaded = response.ok(); } }); - await page.goto('/path-base/inline-assets'); - expect(fontLoaded).toBeTruthy(); }); + + test('includes components dynamically imported in universal load', async ({ + page, + get_computed_style + }) => { + let loaded_css = false; + page.on('response', (response) => { + if (response.url().endsWith('.css')) { + loaded_css = true; + } + }); + await page.goto('/path-base/inline-assets/dynamic-import'); + await expect(page.locator('p')).toHaveText("I'm dynamically imported"); + expect(loaded_css).toBe(false); + expect(await get_computed_style('p', 'color')).toEqual('rgb(0, 0, 255)'); + }); }); } From fae29dd1f77cfbcaac8b54ed08bfab99c159fefd Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Thu, 17 Apr 2025 20:29:27 +0800 Subject: [PATCH 2/6] map client to server stylesheets using rollup bundle data --- .../src/exports/vite/build/build_server.js | 153 ++++++++++-------- packages/kit/src/exports/vite/index.js | 20 ++- 2 files changed, 98 insertions(+), 75 deletions(-) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index ca3eedc18cb3..a49b246d3990 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -11,65 +11,82 @@ import { basename } from 'node:path'; * @param {import('types').ManifestData} manifest_data * @param {import('vite').Manifest} server_manifest * @param {import('vite').Manifest | null} client_manifest - * @param {import('vite').Rollup.OutputAsset[] | null} css + * @param {import('vite').Rollup.OutputBundle | null} server_bundle + * @param {import('vite').Rollup.RollupOutput['output'] | null} client_bundle * @param {import('types').RecursiveRequired} output_config */ -export function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, css, output_config) { +export function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, server_bundle, client_bundle, output_config) { mkdirp(`${out}/server/nodes`); mkdirp(`${out}/server/stylesheets`); /** @type {Map} */ - const stylesheet_lookup = new Map(); - - if (css) { - /** @type {Set} */ - const client_stylesheets = new Set(); - for (const key in client_manifest) { - client_manifest[key].css?.forEach((filename) => { - client_stylesheets.add(filename); - }); + const stylesheets_to_inline = new Map(); + + if (server_bundle && client_bundle) { + /** @type {Map} */ + const client_stylesheets = new Map(); + for (const chunk of client_bundle) { + if (chunk.type !== 'chunk' || !chunk.viteMetadata?.importedCss.size) { + continue; + } + const css = Array.from(chunk.viteMetadata.importedCss); + for (const id of chunk.moduleIds) { + client_stylesheets.set(id, css ); + } } - /** @type {Map} */ + /** @type {Map} */ const server_stylesheets = new Map(); - manifest_data.nodes.forEach((node, i) => { - if (!node.component || !server_manifest[node.component]) return; - - const { stylesheets } = find_deps(server_manifest, node.component, false); - - if (stylesheets.length) { - server_stylesheets.set(i, stylesheets); + for (const key in server_bundle) { + const chunk = server_bundle[key]; + if (chunk.type !== 'chunk' || !chunk.viteMetadata?.importedCss.size) { + continue; } - }); + const css = Array.from(chunk.viteMetadata.importedCss); + for (const id of chunk.moduleIds) { + server_stylesheets.set(id, css ); + } + } - for (const asset of css) { - // ignore dynamically imported stylesheets since we don't need to inline those - if (!client_stylesheets.has(asset.fileName) || asset.source.length >= kit.inlineStyleThreshold) { + // map server stylesheet name to the client stylesheet name + for (const [id, client_css] of client_stylesheets) { + const server_css = server_stylesheets.get(id); + if (!server_css) { continue; } + client_css.forEach((file, i) => { + stylesheets_to_inline.set(file, server_css[i]); + }) + } - // We know that the names for entry points are numbers. - const [index] = basename(asset.fileName).split('.'); - // There can also be other CSS files from shared components - // for example, which we need to ignore here. - if (isNaN(+index)) continue; - - const file = `${out}/server/stylesheets/${index}.js`; - - // we need to inline the server stylesheet instead of the client one - // so that asset paths are correct on document load - const filenames = server_stylesheets.get(+index); - - if (!filenames) { - throw new Error('This should never happen, but if it does, it means we failed to find the server stylesheet for a node.'); + // filter out stylesheets that should not be inlined + for (const chunk of client_bundle) { + if ( + chunk.type === 'asset' && + chunk.fileName.endsWith('.css') && + chunk.source.length < kit.inlineStyleThreshold + ) { + continue; } + stylesheets_to_inline.delete(chunk.fileName); + } - const sources = filenames.map((filename) => { - return fs.readFileSync(`${out}/server/${filename}`, 'utf-8'); - }); - fs.writeFileSync(file, `// ${filenames.join(', ')}\nexport default ${s(sources.join('\n'))};`); + /** @type {Map} */ + const server_stylesheet_sources = new Map(); + for (const key in server_bundle) { + const chunk = server_bundle[key]; + if (chunk.type === 'asset' && chunk.fileName.endsWith('.css')) { + server_stylesheet_sources.set(chunk.fileName, chunk.source.toString()); + } + } - stylesheet_lookup.set(asset.fileName, index); + // map server stylesheet source to the client stylesheet name + for (const [client_file, server_file] of stylesheets_to_inline) { + const source = server_stylesheet_sources.get(server_file); + if (!source) { + throw new Error(`Server stylesheet source not found for client stylesheet ${client_file}`); + } + stylesheets_to_inline.set(client_file, source); } } @@ -122,8 +139,9 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli const entry_path = `${normalizePath(kit.outDir)}/generated/client-optimized/nodes/${i}.js`; const entry = find_deps(client_manifest, entry_path, true); - // eagerly load stylesheets and fonts imported by the SSR-ed page to avoid FOUC. - // If it is not used during SSR, it can be lazily loaded in the browser. + // eagerly load client stylesheets and fonts imported by the SSR-ed page to avoid FOUC. + // However, if it is not used during SSR (not present in the server manifest), + // then it can be lazily loaded in the browser. /** @type {import('types').AssetDependencies | undefined} */ let component; @@ -138,26 +156,26 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli } /** @type {Set} */ - const css_used_by_server = new Set(); + const eager_css = new Set(); /** @type {Set} */ - const assets_used_by_server = new Set(); + const eager_assets = new Set(); - entry.stylesheet_map.forEach((value, key) => { - // pages and layouts are named as node indexes in the client manifest - // so we need to use the original filename when checking against the server manifest - if (key === entry_path) { - key = node.component ?? key; + entry.stylesheet_map.forEach((value, filepath) => { + // pages and layouts are renamed to node indexes when optimised for the client + // so we use the original filename instead to check against the server manifest + if (filepath === entry_path) { + filepath = node.component ?? filepath; } - if (component?.stylesheet_map.has(key) || universal?.stylesheet_map.has(key)) { - value.css.forEach(file => css_used_by_server.add(file)); - value.assets.forEach(file => assets_used_by_server.add(file)); + if (component?.stylesheet_map.has(filepath) || universal?.stylesheet_map.has(filepath)) { + value.css.forEach(file => eager_css.add(file)); + value.assets.forEach(file => eager_assets.add(file)); } }); imported = entry.imports; - stylesheets = Array.from(css_used_by_server); - fonts = filter_fonts(Array.from(assets_used_by_server)); + stylesheets = Array.from(eager_css); + fonts = filter_fonts(Array.from(eager_assets)); } exports.push( @@ -167,19 +185,26 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli ); /** @type {string[]} */ - const styles = []; + const inline_styles = []; stylesheets.forEach((file) => { - if (stylesheet_lookup.has(file)) { - const index = stylesheet_lookup.get(file); - const name = `stylesheet_${index}`; - imports.push(`import ${name} from '../stylesheets/${index}.js';`); - styles.push(`\t${s(file)}: ${name}`); + if (stylesheets_to_inline.has(file)) { + const [filename] = basename(file).split('.'); + const dest = `${out}/server/stylesheets/${filename}.js`; + const source = stylesheets_to_inline.get(file); + if (!source) { + throw new Error(`Server stylesheet source not found for client stylesheet ${file}`); + } + fs.writeFileSync(dest, `// ${filename}\nexport default ${s(source)};`); + + const name = `stylesheet_${filename}`; + imports.push(`import ${name} from '../stylesheets/${filename}.js';`); + inline_styles.push(`\t${s(file)}: ${name}`); } }); - if (styles.length > 0) { - exports.push(`export const inline_styles = () => ({\n${styles.join(',\n')}\n});`); + if (inline_styles.length > 0) { + exports.push(`export const inline_styles = () => ({\n${inline_styles.join(',\n')}\n});`); } fs.writeFileSync( diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js index c2e445e865d2..3cc60f9a1d6d 100644 --- a/packages/kit/src/exports/vite/index.js +++ b/packages/kit/src/exports/vite/index.js @@ -181,6 +181,7 @@ let manifest_data; * @return {Promise} */ async function kit({ svelte_config }) { + /** @type {import('vite')} */ const vite = await resolve_peer_dependency('vite'); const { kit } = svelte_config; @@ -787,7 +788,7 @@ Tips: */ writeBundle: { sequential: true, - async handler(_options) { + async handler(_options, bundle) { if (secondary_build_started) return; // only run this once const verbose = vite_config.logLevel === 'info'; @@ -828,6 +829,7 @@ Tips: server_manifest, null, null, + null, svelte_config.output ); @@ -852,7 +854,7 @@ Tips: secondary_build_started = true; - const { output } = /** @type {import('vite').Rollup.RollupOutput} */ ( + const { output: client_bundle } = /** @type {import('vite').Rollup.RollupOutput} */ ( await vite.build({ configFile: vite_config.configFile, // CLI args @@ -895,7 +897,7 @@ Tips: imports: [...start.imports, ...app.imports], stylesheets: [...start.stylesheets, ...app.stylesheets], fonts: [...start.fonts, ...app.fonts], - uses_env_dynamic_public: output.some( + uses_env_dynamic_public: client_bundle.some( (chunk) => chunk.type === 'chunk' && chunk.modules[env_dynamic_public] ) }; @@ -944,14 +946,14 @@ Tips: imports: start.imports, stylesheets: start.stylesheets, fonts: start.fonts, - uses_env_dynamic_public: output.some( + uses_env_dynamic_public: client_bundle.some( (chunk) => chunk.type === 'chunk' && chunk.modules[env_dynamic_public] ) }; if (svelte_config.kit.output.bundleStrategy === 'inline') { const style = /** @type {import('rollup').OutputAsset} */ ( - output.find( + client_bundle.find( (chunk) => chunk.type === 'asset' && chunk.names.length === 1 && @@ -966,11 +968,6 @@ Tips: } } - const css = output.filter( - /** @type {(value: any) => value is import('vite').Rollup.OutputAsset} */ - (value) => value.type === 'asset' && value.fileName.endsWith('.css') - ); - // regenerate manifest now that we have client entry... fs.writeFileSync( manifest_path, @@ -989,7 +986,8 @@ Tips: manifest_data, server_manifest, client_manifest, - css, + bundle, + client_bundle, svelte_config.kit.output ); From 22c0e1128c5c7d4887b40555448499df1297e3c2 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Thu, 17 Apr 2025 20:40:00 +0800 Subject: [PATCH 3/6] changeset --- .changeset/spicy-cougars-wave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spicy-cougars-wave.md diff --git a/.changeset/spicy-cougars-wave.md b/.changeset/spicy-cougars-wave.md new file mode 100644 index 000000000000..d65ca6f9e120 --- /dev/null +++ b/.changeset/spicy-cougars-wave.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: correctly inline stylesheets of components dynamically imported in a universal load function if they are below the configured inlineStyleThreshold From 4a74ec488952121b6f630aa16daa0cef2b7f35a8 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Thu, 17 Apr 2025 20:48:06 +0800 Subject: [PATCH 4/6] avoid duplicate filenames --- packages/kit/src/exports/vite/build/build_server.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index a49b246d3990..cc052dcc0003 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -187,9 +187,9 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli /** @type {string[]} */ const inline_styles = []; - stylesheets.forEach((file) => { + stylesheets.forEach((file, i) => { if (stylesheets_to_inline.has(file)) { - const [filename] = basename(file).split('.'); + const filename = basename(file); const dest = `${out}/server/stylesheets/${filename}.js`; const source = stylesheets_to_inline.get(file); if (!source) { @@ -197,7 +197,7 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli } fs.writeFileSync(dest, `// ${filename}\nexport default ${s(source)};`); - const name = `stylesheet_${filename}`; + const name = `stylesheet_${i}`; imports.push(`import ${name} from '../stylesheets/${filename}.js';`); inline_styles.push(`\t${s(file)}: ${name}`); } From d684783d5b36917b09b555923af78bcc5e7f5c9a Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Wed, 23 Apr 2025 12:07:27 +0800 Subject: [PATCH 5/6] skip bundle mapping if inline style threshold is not enabled --- packages/kit/src/exports/vite/build/build_server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index cc052dcc0003..55b5899d6dc0 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -22,7 +22,7 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli /** @type {Map} */ const stylesheets_to_inline = new Map(); - if (server_bundle && client_bundle) { + if (server_bundle && client_bundle && kit.inlineStyleThreshold > 0) { /** @type {Map} */ const client_stylesheets = new Map(); for (const chunk of client_bundle) { From 50bd864e551cee10fe73c8ba4a0864571f4eab75 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Wed, 23 Apr 2025 13:16:57 +0800 Subject: [PATCH 6/6] optimise mapping and skip if no inline threshold --- packages/kit/src/exports/public.d.ts | 2 +- .../src/exports/vite/build/build_server.js | 92 +++++++++---------- packages/kit/src/exports/vite/index.js | 10 +- packages/kit/types/index.d.ts | 2 +- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/packages/kit/src/exports/public.d.ts b/packages/kit/src/exports/public.d.ts index 732df205d0ae..6efab36ef8eb 100644 --- a/packages/kit/src/exports/public.d.ts +++ b/packages/kit/src/exports/public.d.ts @@ -468,7 +468,7 @@ export interface KitConfig { errorTemplate?: string; }; /** - * Inline CSS inside a `