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

fix: inline server stylesheets instead of client stylesheets #13068

Merged
merged 9 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/sixty-days-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: correctly link to assets inlined by the `inlineStyleThreshold` option
42 changes: 36 additions & 6 deletions packages/kit/src/exports/vite/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mkdirp } from '../../../utils/filesystem.js';
import { find_deps, resolve_symlinks } from './utils.js';
import { s } from '../../../utils/misc.js';
import { normalizePath } from 'vite';
import { basename } from 'node:path';

/**
* @param {string} out
Expand All @@ -17,18 +18,47 @@ export function build_server_nodes(out, kit, manifest_data, server_manifest, cli
mkdirp(`${out}/server/nodes`);
mkdirp(`${out}/server/stylesheets`);

/** @type {Map<string, string>} */
const stylesheet_lookup = new Map();

if (css) {
css.forEach((asset) => {
if (asset.source.length < kit.inlineStyleThreshold) {
const index = stylesheet_lookup.size;
const file = `${out}/server/stylesheets/${index}.js`;
/** @type {Set<string>} */
const client_stylesheets = new Set();
for (const key in client_manifest) {
const file = client_manifest[key];
if (file.css?.[0]) {
client_stylesheets.add(file.css[0]);
}
}

/** @type {Map<number, string>} */
const server_stylesheets = new Map();

fs.writeFileSync(file, `// ${asset.fileName}\nexport default ${s(asset.source)};`);
stylesheet_lookup.set(asset.fileName, index);
const component_stylesheet_map = new Map(Object.values(server_manifest).map((file) => [file.src, file.css?.[0]]));

manifest_data.nodes.forEach((node, i) => {
const server_stylesheet = component_stylesheet_map.get(node.component);
if (node.component && server_stylesheet) {
server_stylesheets.set(i, server_stylesheet);
}
});

// ignore dynamically imported stylesheets since we can't inline those
css.filter(asset => client_stylesheets.has(asset.fileName))
Copy link
Member

Choose a reason for hiding this comment

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

Just for understanding: asset.fileName is always different between client and server, i.e. there's no chance of this accidentally filtering out a server stylesheet?

Copy link
Member Author

@eltigerchino eltigerchino Dec 17, 2024

Choose a reason for hiding this comment

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

Yeah, they're always different. The client stylesheets correspond to the node index number while server stylesheets are always "_page" or "_layout".

Copy link
Contributor

@tkhs0813 tkhs0813 Jan 29, 2025

Choose a reason for hiding this comment

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

@eltigerchino @dummdidumm
When running npm run build in SvelteKit v2.16.1, the following error occurs:

[vite-plugin-sveltekit-compile] ENOENT: no such file or directory, open '/Users/me/.svelte-kit/output/server/undefined'
    at Object.readFileSync (node:fs:442:20)
    at file:///Users/me/node_modules/@sveltejs/kit/src/exports/vite/build/build_server.js:59:24
    at Array.forEach (<anonymous>)
    at build_server_nodes (file:///Users/me/node_modules/@sveltejs/kit/src/exports/vite/build/build_server.js:51:5)
    at Object.handler (file:///Users/me/node_modules/@sveltejs/kit/src/exports/vite/index.js:919:5)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async PluginDriver.hookParallel (file:///Users/me/node_modules/rollup/dist/es/shared/node-entry.js:20734:17)
    at async file:///Users/me/node_modules/rollup/dist/es/shared/node-entry.js:21783:13
    at async catchUnfinishedHookActions (file:///Users/me/node_modules/rollup/dist/es/shared/node-entry.js:21158:16)
    at async build (file:///Users/me/node_modules/vite/dist/node/chunks/dep-CHZK6zbr.js:65607:16)

Looking at the contents of client_stylesheets and asset.fileName, there is a filename string like _app/immutable/assets/Header.k7gKBesC.css, which seems to be causing the error.
inlineStyleThreshold is set in sveltekit.config.js.

Since this issue only occurs in the SvelteKit application developed at my company, I am unable to provide a minimal reproducible code example. Do you know how to resolve this?

Using the following versions:

    "@sveltejs/adapter-node": "5.2.12",
    "@sveltejs/kit": "2.16.1",
    "@sveltejs/vite-plugin-svelte": "3.1.2",
    "vite": "5.4.14",

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that someone else has encountered the same issue as me.
#13304 (comment)

Choose a reason for hiding this comment

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

@tkhs0813, removing inlineStyleThreshold resolved the issue for me!

Copy link
Member

Choose a reason for hiding this comment

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

Will be fixed by #13395

Choose a reason for hiding this comment

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

@dummdidumm, great! Thanks for the update!

.forEach((asset) => {
if (asset.source.length < kit.inlineStyleThreshold) {
const [index] = basename(asset.fileName).split('.');
const server_stylesheet = server_stylesheets.get(+index);
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 source = fs.readFileSync(`${out}/server/${server_stylesheet}`, 'utf-8');

fs.writeFileSync(file, `// ${server_stylesheet}\nexport default ${s(source)};`);
stylesheet_lookup.set(asset.fileName, index);
}
});
}

manifest_data.nodes.forEach((node, i) => {
Expand Down
1 change: 1 addition & 0 deletions packages/kit/test/apps/options/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"test:build": "playwright test"
},
"devDependencies": {
"@fontsource/libre-barcode-128-text": "^5.1.0",
"@sveltejs/kit": "workspace:^",
"@sveltejs/vite-plugin-svelte": "^5.0.1",
"cross-env": "^7.0.3",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import '@fontsource/libre-barcode-128-text';
</script>

<p>Hello world!</p>

<style>
p {
font-family: 'Libre Barcode 128 Text', sans-serif;
}
</style>
16 changes: 16 additions & 0 deletions packages/kit/test/apps/options/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,22 @@ if (!process.env.DEV) {
expect(await page.content()).not.toMatch('navigator.serviceWorker');
});
});

test.describe('inlineStyleThreshold', () => {
test('loads asset', 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.describe('Vite options', () => {
Expand Down
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading