From 098cdebe950ca08fde99d70019f483a9ec50942a Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 31 Mar 2026 09:31:50 +0200 Subject: [PATCH] fix: disallow css injected with custom renderer --- .../98-reference/.generated/compile-errors.md | 6 ++++ .../svelte/messages/compile-errors/options.md | 4 +++ packages/svelte/src/compiler/errors.js | 9 ++++++ .../compiler/phases/1-parse/read/options.js | 8 ++++++ .../src/compiler/phases/2-analyze/index.js | 5 ++++ .../svelte/src/internal/client/dom/css.js | 13 ++------- .../src/internal/client/dom/operations.js | 28 ------------------- .../css-injected-compiler-option/_config.js | 9 ++++++ .../css-injected-compiler-option/main.svelte | 7 +++++ .../samples/css-injected-mixed/_config.js | 6 ++++ .../samples/css-injected-mixed/main.svelte | 9 ++++++ .../css-injected-svelte-options/_config.js | 6 ++++ .../css-injected-svelte-options/main.svelte | 9 ++++++ .../svelte/tests/custom-renderers/shared.ts | 26 +++++++++++++++-- 14 files changed, 104 insertions(+), 41 deletions(-) create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/_config.js create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/main.svelte create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-mixed/_config.js create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-mixed/main.svelte create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/_config.js create mode 100644 packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/main.svelte diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index 01b2dd1494..43493993b8 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -689,6 +689,12 @@ HTML restricts where certain elements can appear. In case of a violation the bro - `` will result in `` (the `
` is removed) - `
cell
` will result in `
cell
` (a `` is auto-inserted) +### options_css_injected_with_custom_renderer + +``` +`css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection +``` + ### options_invalid_value ``` diff --git a/packages/svelte/messages/compile-errors/options.md b/packages/svelte/messages/compile-errors/options.md index 0709f0d3f6..f841aacd2b 100644 --- a/packages/svelte/messages/compile-errors/options.md +++ b/packages/svelte/messages/compile-errors/options.md @@ -1,3 +1,7 @@ +## options_css_injected_with_custom_renderer + +> `css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection + ## options_invalid_value > Invalid compiler option: %details% diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 976925d181..b97879ccee 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -47,6 +47,15 @@ function e(node, code, message) { throw new InternalCompileError(code, message, start !== undefined ? [start, end ?? start] : undefined); } +/** + * `css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection + * @param {null | number | NodeLike} node + * @returns {never} + */ +export function options_css_injected_with_custom_renderer(node) { + e(node, 'options_css_injected_with_custom_renderer', `\`css: "injected"\` is not compatible with \`customRenderer\` — custom renderers do not support CSS injection\nhttps://svelte.dev/e/options_css_injected_with_custom_renderer`); +} + /** * Invalid compiler option: %details% * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/1-parse/read/options.js b/packages/svelte/src/compiler/phases/1-parse/read/options.js index dc14bd47f8..a55a23357e 100644 --- a/packages/svelte/src/compiler/phases/1-parse/read/options.js +++ b/packages/svelte/src/compiler/phases/1-parse/read/options.js @@ -197,6 +197,14 @@ export default function read_options(node) { } } + if (component_options.css === 'injected' && component_options.customRenderer !== undefined) { + // Find the css attribute node for the error position + const css_attribute = node.attributes.find( + (/** @type {any} */ a) => a.type === 'Attribute' && a.name === 'css' + ); + e.options_css_injected_with_custom_renderer(css_attribute ?? node); + } + return component_options; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 6dce87bb99..75cb299e2f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -468,6 +468,11 @@ export function analyze_component(root, source, options) { const custom_element_from_option = options.customElement({ filename: options.filename }); const css = options.css({ filename: options.filename }); const custom_renderer = options.experimental.customRenderer?.({ filename: options.filename }); + + if (css === 'injected' && custom_renderer !== undefined) { + e.options_css_injected_with_custom_renderer(null); + } + const custom_element = options.customElementOptions ?? custom_element_from_option; const is_custom_element = !!options.customElementOptions || custom_element_from_option; diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index 81256f504d..c16e7fc48f 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,23 +1,16 @@ import { DEV } from 'esm-env'; import { register_style } from '../dev/css.js'; import { effect } from '../reactivity/effects.js'; -import { - create_element, - get_root_node, - query_selector, - append_child, - set_text_content -} from './operations.js'; +import { create_element, append_child, set_text_content } from './operations.js'; /** * @param {Node} anchor * @param {{ hash: string, code: string }} css */ export function append_styles(anchor, css) { - // TODO RENDERER: disallow css inject with custom renderer? // Use `queue_micro_task` to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results effect(() => { - var root = get_root_node(anchor); + var root = anchor.getRootNode(); // TODO: DOM access var target = /** @type {ShadowRoot} */ (root).host @@ -27,7 +20,7 @@ export function append_styles(anchor, css) { // Always querying the DOM is roughly the same perf as additionally checking for presence in a map first assuming // that you'll get cache hits half of the time, so we just always query the dom for simplicity and code savings. - if (!query_selector(/** @type {Element} */ (target), '#' + css.hash)) { + if (!(/** @type {Element} */ (target).querySelector('#' + css.hash))) { const style = create_element('style'); // TODO: DOM access style.id = css.hash; diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 065b0f51ab..f18b06d3e0 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -151,7 +151,6 @@ export function first_child(node, is_text = false) { var first = get_first_child(node); // TODO prevent user comments with the empty string when preserveComments is true - // TODO RENDERER: can we find another way to not have an `isComment` on the renderer? if (is_comment(first) && get_node_value(first) === '') return get_next_sibling(first); return first; @@ -642,8 +641,6 @@ export function remove_attribute(element, name) { * @returns {boolean} */ export function has_attribute(element, name) { - // TODO RENDERER: could be worth removing and just using get? - if (renderer) return renderer.hasAttribute(element, name); return element.hasAttribute(name); } @@ -792,42 +789,17 @@ export function class_list_toggle(element, name, force) { element.classList.toggle(name, force); } -/** - * @param {Element} element - * @param {string} selector - */ -export function query_selector(element, selector) { - if (renderer) { - // TODO RENDERER: just used with css inject which doesn't make sense with custom renderers - throw new Error('querySelector is not supported with custom renderers'); - } - return element.querySelector(selector); -} - /** * @param {Element | DocumentFragment} element * @param {string} selector */ export function query_selector_all(element, selector) { if (renderer) { - // TODO RENDERER: just used with css inject which doesn't make sense with custom renderers throw new Error('querySelectorAll is not supported with custom renderers'); } return element.querySelectorAll(selector); } -/** - * @param {Node} node - * @returns {Node} - */ -export function get_root_node(node) { - if (renderer) { - // TODO RENDERER: just used with css inject which doesn't make sense with custom renderers - throw new Error('getRootNode is not supported with custom renderers'); - } - return node.getRootNode(); -} - /** * @param {HTMLElement} element */ diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/_config.js b/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/_config.js new file mode 100644 index 0000000000..963218a3fb --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/_config.js @@ -0,0 +1,9 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + css: 'injected' + }, + compile_error: + '`css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection' +}); diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/main.svelte b/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/main.svelte new file mode 100644 index 0000000000..4f4c13a0f4 --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-compiler-option/main.svelte @@ -0,0 +1,7 @@ +

hello

+ + diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/_config.js b/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/_config.js new file mode 100644 index 0000000000..5b7e622cc7 --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/_config.js @@ -0,0 +1,6 @@ +import { test } from '../../test'; + +export default test({ + compile_error: + '`css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection' +}); diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/main.svelte b/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/main.svelte new file mode 100644 index 0000000000..0245f4212f --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-mixed/main.svelte @@ -0,0 +1,9 @@ + + +

hello

+ + diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/_config.js b/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/_config.js new file mode 100644 index 0000000000..5b7e622cc7 --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/_config.js @@ -0,0 +1,6 @@ +import { test } from '../../test'; + +export default test({ + compile_error: + '`css: "injected"` is not compatible with `customRenderer` — custom renderers do not support CSS injection' +}); diff --git a/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/main.svelte b/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/main.svelte new file mode 100644 index 0000000000..4f19dc820c --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/css-injected-svelte-options/main.svelte @@ -0,0 +1,9 @@ + + +

hello

+ + diff --git a/packages/svelte/tests/custom-renderers/shared.ts b/packages/svelte/tests/custom-renderers/shared.ts index b682cd7819..0f496257aa 100644 --- a/packages/svelte/tests/custom-renderers/shared.ts +++ b/packages/svelte/tests/custom-renderers/shared.ts @@ -12,6 +12,7 @@ export interface CustomRendererTest extends BaseTest { compileOptions?: Partial; props?: Record; error?: string; + compile_error?: string; runtime_error?: string; warnings?: string[]; test?: (args: { @@ -35,7 +36,7 @@ const console_warn = console.warn; const renderer_path = path.resolve(import.meta.dirname, 'renderer.js'); export function custom_renderer_suite() { - return suite_with_variants( + return suite_with_variants( ['custom-renderer'], (_variant, _config) => { return false; @@ -44,12 +45,23 @@ export function custom_renderer_suite() { return common_setup(cwd, config); }, async (config, cwd, _variant, common) => { + if (common === null) { + // compile_error was expected and matched — test passed + return; + } + if (config.compile_error) { + // compile_error was expected but common_setup didn't throw + assert.fail('Expected a compile error'); + } await run_test(cwd, config, common); } ); } -async function common_setup(cwd: string, config: CustomRendererTest) { +async function common_setup( + cwd: string, + config: CustomRendererTest +): Promise { const compile_options: CompileOptions = { generate: 'client', rootDir: cwd, @@ -61,7 +73,15 @@ async function common_setup(cwd: string, config: CustomRendererTest) { } }; - await compile_directory(cwd, 'client', compile_options); + try { + await compile_directory(cwd, 'client', compile_options); + } catch (err) { + if (config.compile_error) { + assert.include((err as Error).message, config.compile_error); + return null; + } + throw err; + } return compile_options; }