From 010620498287a580acee26ccb0b1fb635a67c450 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:47:21 +0100 Subject: [PATCH] fix: minify inject CSS in prod mode (#14006) When CSS is externalized we rightfully rely on the following tooling chain to properly minify CSS. When we inject the CSS however, that tooling won't be able to do that, so we gotta do it ourselves. This PR brings back most of that logic that existed in Svelte 4. Fixes #13716 --- .changeset/red-berries-watch.md | 5 + .../compiler/phases/3-transform/css/index.js | 93 +++++++++++++++---- .../css-injected-options-nested/Nested.svelte | 5 + .../_expected.html | 2 +- .../_expected_head.html | 6 +- .../css-injected-options/_expected_head.html | 6 +- .../samples/css/_expected_head.html | 6 +- 7 files changed, 90 insertions(+), 33 deletions(-) create mode 100644 .changeset/red-berries-watch.md diff --git a/.changeset/red-berries-watch.md b/.changeset/red-berries-watch.md new file mode 100644 index 0000000000..92460c9cd0 --- /dev/null +++ b/.changeset/red-berries-watch.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: minify inject CSS in prod mode diff --git a/packages/svelte/src/compiler/phases/3-transform/css/index.js b/packages/svelte/src/compiler/phases/3-transform/css/index.js index decdb21ad7..6f87b04850 100644 --- a/packages/svelte/src/compiler/phases/3-transform/css/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/css/index.js @@ -11,6 +11,7 @@ import { dev } from '../../../state.js'; * @typedef {{ * code: MagicString; * hash: string; + * minify: boolean; * selector: string; * keyframes: string[]; * specificity: { @@ -32,6 +33,7 @@ export function render_stylesheet(source, analysis, options) { const state = { code, hash: analysis.css.hash, + minify: analysis.inject_styles && !options.dev, selector: `.${analysis.css.hash}`, keyframes: analysis.css.keyframes, specificity: { @@ -45,6 +47,9 @@ export function render_stylesheet(source, analysis, options) { code.remove(0, ast.content.start); code.remove(/** @type {number} */ (ast.content.end), source.length); + if (state.minify) { + remove_preceeding_whitespace(ast.content.end, state); + } const css = { code: code.toString(), @@ -116,22 +121,47 @@ const visitors = { index++; } + } else if (state.minify) { + remove_preceeding_whitespace(node.start, state); + + // Don't minify whitespace in custom properties, since some browsers (Chromium < 99) + // treat --foo: ; and --foo:; differently + if (!node.property.startsWith('--')) { + let start = node.start + node.property.length + 1; + let end = start; + while (/\s/.test(state.code.original[end])) end++; + if (end > start) state.code.remove(start, end); + } } }, Rule(node, { state, next, visit }) { + if (state.minify) { + remove_preceeding_whitespace(node.start, state); + remove_preceeding_whitespace(node.block.end - 1, state); + } + // keep empty rules in dev, because it's convenient to // see them in devtools if (!dev && is_empty(node)) { - state.code.prependRight(node.start, '/* (empty) '); - state.code.appendLeft(node.end, '*/'); - escape_comment_close(node, state.code); + if (state.minify) { + state.code.remove(node.start, node.end); + } else { + state.code.prependRight(node.start, '/* (empty) '); + state.code.appendLeft(node.end, '*/'); + escape_comment_close(node, state.code); + } + return; } if (!is_used(node)) { - state.code.prependRight(node.start, '/* (unused) '); - state.code.appendLeft(node.end, '*/'); - escape_comment_close(node, state.code); + if (state.minify) { + state.code.remove(node.start, node.end); + } else { + state.code.prependRight(node.start, '/* (unused) '); + state.code.appendLeft(node.end, '*/'); + escape_comment_close(node, state.code); + } return; } @@ -141,11 +171,16 @@ const visitors = { if (selector.children.length === 1 && selector.children[0].selectors.length === 1) { // `:global {...}` - state.code.prependRight(node.start, '/* '); - state.code.appendLeft(node.block.start + 1, '*/'); + if (state.minify) { + state.code.remove(node.start, node.block.start + 1); + state.code.remove(node.block.end - 1, node.end); + } else { + state.code.prependRight(node.start, '/* '); + state.code.appendLeft(node.block.start + 1, '*/'); - state.code.prependRight(node.block.end - 1, '/*'); - state.code.appendLeft(node.block.end, '*/'); + state.code.prependRight(node.block.end - 1, '/*'); + state.code.appendLeft(node.block.end, '*/'); + } // don't recurse into selector or body return; @@ -162,7 +197,8 @@ const visitors = { // Only add comments if we're not inside a complex selector that itself is unused if (!path.find((n) => n.type === 'ComplexSelector' && !n.metadata.used)) { let pruning = false; - let last = node.children[0].start; + let prune_start = node.children[0].start; + let last = prune_start; for (let i = 0; i < node.children.length; i += 1) { const selector = node.children[i]; @@ -172,12 +208,20 @@ const visitors = { let i = selector.start; while (state.code.original[i] !== ',') i--; - state.code.overwrite(i, i + 1, '*/'); - } else { - if (i === 0) { - state.code.prependRight(selector.start, '/* (unused) '); + if (state.minify) { + state.code.remove(prune_start, i + 1); } else { - state.code.overwrite(last, selector.start, ' /* (unused) '); + state.code.overwrite(i, i + 1, '*/'); + } + } else { + prune_start = selector.start; + + if (!state.minify) { + if (i === 0) { + state.code.prependRight(selector.start, '/* (unused) '); + } else { + state.code.overwrite(last, selector.start, ' /* (unused) '); + } } } @@ -188,7 +232,11 @@ const visitors = { } if (pruning) { - state.code.appendLeft(last, '*/'); + if (state.minify) { + state.code.remove(prune_start, last); + } else { + state.code.appendLeft(last, '*/'); + } } } @@ -320,6 +368,17 @@ const visitors = { } }; +/** + * Walk backwards until we find a non-whitespace character + * @param {number} end + * @param {State} state + */ +function remove_preceeding_whitespace(end, state) { + let start = end; + while (/\s/.test(state.code.original[start - 1])) start--; + if (start < end) state.code.remove(start, end); +} + /** @param {Css.Rule} rule */ function is_empty(rule) { if (rule.metadata.is_global_block) { diff --git a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/Nested.svelte b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/Nested.svelte index 81243bf114..7b6038eef3 100644 --- a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/Nested.svelte +++ b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/Nested.svelte @@ -6,4 +6,9 @@ .bar { color: red; } + .unused { + .also-unused { + color: green; + } + } diff --git a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected.html b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected.html index 1f0b2b95fe..3069a50ad7 100644 --- a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected.html +++ b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected.html @@ -1 +1 @@ -
bar
foo
\ No newline at end of file +
bar
foo
\ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected_head.html index 806f6c2db2..6474fde317 100644 --- a/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/css-injected-options-nested/_expected_head.html @@ -1,5 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/css-injected-options/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/css-injected-options/_expected_head.html index 7281935646..9c4f8a8538 100644 --- a/packages/svelte/tests/server-side-rendering/samples/css-injected-options/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/css-injected-options/_expected_head.html @@ -1,5 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/css/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/css/_expected_head.html index 7281935646..9c4f8a8538 100644 --- a/packages/svelte/tests/server-side-rendering/samples/css/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/css/_expected_head.html @@ -1,5 +1 @@ - \ No newline at end of file + \ No newline at end of file