From db68a36b6fd912e48c4688bf986fdd9e35f5bf12 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 May 2024 17:17:38 +0200 Subject: [PATCH 1/7] chore: backwards compat for dash-case warnings Compiler warnings that are dashed are also taken into account for ignoring warnings for backwards compatiblity (else many apps would be littered with a11y warnings now). Also adds logic to the migration script to convert these to underscores. --- .changeset/yellow-guests-double.md | 5 ++++ .../templates/compile-warnings.js | 3 ++ packages/svelte/src/compiler/migrate/index.js | 28 +++++++++++++++++++ packages/svelte/src/compiler/warnings.js | 3 ++ .../samples/svelte-ignore/input.svelte | 7 +++++ .../samples/svelte-ignore/output.svelte | 7 +++++ 6 files changed, 53 insertions(+) create mode 100644 .changeset/yellow-guests-double.md create mode 100644 packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte diff --git a/.changeset/yellow-guests-double.md b/.changeset/yellow-guests-double.md new file mode 100644 index 000000000000..454caacdba22 --- /dev/null +++ b/.changeset/yellow-guests-double.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +chore: backwards compat for dash-case warnings diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index cac547de6049..257de05db5f1 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -32,6 +32,9 @@ export function reset_warnings(options) { function w(node, code, message) { // @ts-expect-error if (node?.ignores?.has(code)) return; + // backwards compat: Svelte 5 changed all warnings from dash to underscore + // @ts-expect-error + if (node?.ignores?.has(code.replaceAll('-', '_'))) return; warnings.push({ code, diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index e6328c70baff..8fe0ae68b0b6 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -7,6 +7,7 @@ import { get_rune } from '../phases/scope.js'; import { reset_warnings } from '../warnings.js'; import { extract_identifiers } from '../utils/ast.js'; import { regex_is_valid_identifier } from '../phases/patterns.js'; +import { extract_svelte_ignore } from '../utils/extract_svelte_ignore.js'; /** * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. @@ -174,6 +175,23 @@ export function migrate(source) { /** @type {import('zimmerframe').Visitors} */ const instance_script = { + _(node, { state, next }) { + // @ts-expect-error + const comments = node.leadingComments; + if (comments) { + for (const comment of comments) { + if (comment.type === 'Line' && extract_svelte_ignore(comment.value).length > 0) { + const svelteIgnore = comment.value.indexOf('svelte-ignore'); + state.str.overwrite( + comment.start + svelteIgnore + 13 + '//'.length, + comment.end, + comment.value.substring(svelteIgnore + 13).replaceAll('-', '_') + ); + } + } + } + next(); + }, Identifier(node, { state }) { handle_identifier(node, state); }, @@ -474,6 +492,16 @@ const template = { } else { state.str.update(node.start, node.end, `{@render ${name}?.(${slot_props})}`); } + }, + Comment(node, { state }) { + if (extract_svelte_ignore(node.data).length > 0) { + const svelteIgnore = node.data.indexOf('svelte-ignore'); + state.str.overwrite( + node.start + svelteIgnore + 13 + ''.length, + node.data.substring(svelteIgnore + 13).replaceAll('-', '_') + ); + } } }; diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 39d9d9efb0b8..cc5e8660df06 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -30,6 +30,9 @@ export function reset_warnings(options) { function w(node, code, message) { // @ts-expect-error if (node?.ignores?.has(code)) return; + // backwards compat: Svelte 5 changed all warnings from dash to underscore + // @ts-expect-error + if (node?.ignores?.has(code.replaceAll('-', '_'))) return; warnings.push({ code, diff --git a/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte b/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte new file mode 100644 index 000000000000..b065e7c49802 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte @@ -0,0 +1,7 @@ + + + +
\ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte b/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte new file mode 100644 index 000000000000..42c62798d2b2 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte @@ -0,0 +1,7 @@ + + + +
\ No newline at end of file From 2a3ca2cacde41b0b3b05e6d6dcaed5bc384e68f7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 May 2024 23:30:42 +0200 Subject: [PATCH 2/7] fix uncovered bug with attribute a11y warnings being unsilenceable --- .../process-messages/templates/compile-warnings.js | 2 +- .../svelte/src/compiler/phases/2-analyze/a11y.js | 6 ++++++ packages/svelte/src/compiler/warnings.js | 2 +- .../input.svelte | 12 ++++++++++++ .../warnings.json | 1 + .../validator/samples/ignore-warning/input.svelte | 3 +++ .../validator/samples/ignore-warning/warnings.json | 4 ++-- 7 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte create mode 100644 packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/warnings.json diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index 257de05db5f1..53eaeb43216d 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -34,7 +34,7 @@ function w(node, code, message) { if (node?.ignores?.has(code)) return; // backwards compat: Svelte 5 changed all warnings from dash to underscore // @ts-expect-error - if (node?.ignores?.has(code.replaceAll('-', '_'))) return; + if (node?.ignores?.has(code.replaceAll('_', '-'))) return; warnings.push({ code, diff --git a/packages/svelte/src/compiler/phases/2-analyze/a11y.js b/packages/svelte/src/compiler/phases/2-analyze/a11y.js index a45c3c1f97ea..5e6e05ab01be 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/a11y.js +++ b/packages/svelte/src/compiler/phases/2-analyze/a11y.js @@ -702,6 +702,12 @@ function check_element(node, state) { let has_contenteditable_binding = false; for (const attribute of node.attributes) { + // gross but necessary: the visitor that sets this isn't called in time + if (state.ignores.size > 0) { + // @ts-expect-error + attribute.ignores = state.ignores; + } + if (attribute.type === 'SpreadAttribute') { has_spread = true; } else if (attribute.type === 'OnDirective') { diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index cc5e8660df06..8cd855af854c 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -32,7 +32,7 @@ function w(node, code, message) { if (node?.ignores?.has(code)) return; // backwards compat: Svelte 5 changed all warnings from dash to underscore // @ts-expect-error - if (node?.ignores?.has(code.replaceAll('-', '_'))) return; + if (node?.ignores?.has(code.replaceAll('_', '-'))) return; warnings.push({ code, diff --git a/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte new file mode 100644 index 000000000000..9a25e77ac13f --- /dev/null +++ b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte @@ -0,0 +1,12 @@ + + + +
+ +
+ + +
diff --git a/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/warnings.json b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/warnings.json new file mode 100644 index 000000000000..fe51488c7066 --- /dev/null +++ b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/warnings.json @@ -0,0 +1 @@ +[] diff --git a/packages/svelte/tests/validator/samples/ignore-warning/input.svelte b/packages/svelte/tests/validator/samples/ignore-warning/input.svelte index cfbe0e538daa..7a2328e57c43 100644 --- a/packages/svelte/tests/validator/samples/ignore-warning/input.svelte +++ b/packages/svelte/tests/validator/samples/ignore-warning/input.svelte @@ -4,4 +4,7 @@ but this is still discouraged + +
+ diff --git a/packages/svelte/tests/validator/samples/ignore-warning/warnings.json b/packages/svelte/tests/validator/samples/ignore-warning/warnings.json index 926543bf3ff0..3997af9dbde7 100644 --- a/packages/svelte/tests/validator/samples/ignore-warning/warnings.json +++ b/packages/svelte/tests/validator/samples/ignore-warning/warnings.json @@ -15,12 +15,12 @@ "code": "a11y_missing_attribute", "end": { "column": 22, - "line": 7 + "line": 10 }, "message": "`` element should have an alt attribute", "start": { "column": 0, - "line": 7 + "line": 10 } } ] From 4a7c0b8afefb9439ea994f97480e401638c4609e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 May 2024 23:35:09 +0200 Subject: [PATCH 3/7] breaking change documentation --- .../03-appendix/02-breaking-changes.md | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md index 9cb885555b4b..8c4a2e0894b8 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md @@ -133,6 +133,23 @@ In Svelte 4 syntax, every property (declared via `export let`) is bindable, mean Setting the `accessors` option to `true` makes properties of a component directly accessible on the component instance. In runes mode, properties are never accessible on the component instance. You can use component exports instead if you need to expose them. +### Props may update with same value + +Svelte 4 did safe equality checks on properties by default. That meant that even if you update a value, and part of of tha value is a primitive passed as a property, and that part did not change, it would not trigger a property update. + +```svelte + + + +``` + +In Svelte 5, the above would trigger an update in the child component. Note that Svelte 4 by default did always update properties if they were not primitives (i.e. objects or arrays for example). + ## Other breaking changes ### Stricter `@const` assignment validation @@ -158,9 +175,9 @@ In the event that you need to support ancient browsers that don't implement `:wh css = css.replace(/:where\((.+?)\)/, '$1'); ``` -### Renames of various error/warning codes +### Renames of error/warning codes -Various error and warning codes have been renamed slightly. +Error and warning codes have been renamed: Instead of using dashes they now use underscores as separators (e.g. `a11y_misplaced_scope` instead of `a11y-misplaced-scope`). Some codes have also been slightly reworded. For backwards compatibility, dash separators are still supported, but you should migrate towards underscores. ### Reduced number of namespaces From eb7cac0fde1ab41522a28436a745ddee0965ed27 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 May 2024 23:41:22 +0200 Subject: [PATCH 4/7] woops --- packages/svelte/src/compiler/migrate/index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 8fe0ae68b0b6..12e5085dbb22 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -181,11 +181,11 @@ const instance_script = { if (comments) { for (const comment of comments) { if (comment.type === 'Line' && extract_svelte_ignore(comment.value).length > 0) { - const svelteIgnore = comment.value.indexOf('svelte-ignore'); + const svelte_ignore = comment.value.indexOf('svelte-ignore'); state.str.overwrite( - comment.start + svelteIgnore + 13 + '//'.length, + comment.start + svelte_ignore + 13 + '//'.length, comment.end, - comment.value.substring(svelteIgnore + 13).replaceAll('-', '_') + comment.value.substring(svelte_ignore + 13).replaceAll('-', '_') ); } } @@ -495,11 +495,11 @@ const template = { }, Comment(node, { state }) { if (extract_svelte_ignore(node.data).length > 0) { - const svelteIgnore = node.data.indexOf('svelte-ignore'); + const svelte_ignore = node.data.indexOf('svelte-ignore'); state.str.overwrite( - node.start + svelteIgnore + 13 + ''.length, - node.data.substring(svelteIgnore + 13).replaceAll('-', '_') + node.data.substring(svelte_ignore + 13).replaceAll('-', '_') ); } } From 6d1fbf78e1b3484f30ab6794bc2e00f24a7c1457 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 8 May 2024 21:12:14 +0200 Subject: [PATCH 5/7] backwards compat in legacy mode, no backwards compat in runes mode. Also adjusted the approach to ignores. --- .../templates/compile-warnings.js | 73 ++++++++++++++--- packages/svelte/src/compiler/index.js | 10 +-- .../src/compiler/phases/2-analyze/a11y.js | 6 -- .../src/compiler/phases/2-analyze/index.js | 76 +++++++----------- .../src/compiler/phases/2-analyze/types.d.ts | 1 - packages/svelte/src/compiler/warnings.js | 79 ++++++++++++++++--- .../input.svelte | 6 +- .../ignore-warning-dash-runes/input.svelte | 9 +++ .../ignore-warning-dash-runes/warnings.json | 26 ++++++ 9 files changed, 200 insertions(+), 86 deletions(-) create mode 100644 packages/svelte/tests/validator/samples/ignore-warning-dash-runes/input.svelte create mode 100644 packages/svelte/tests/validator/samples/ignore-warning-dash-runes/warnings.json diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index 53eaeb43216d..3c9c8d13febe 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -2,9 +2,12 @@ import { getLocator } from 'locate-character'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -/** @type {import('#compiler').Warning[]} */ +/** @type {Array<{ warning: import('#compiler').Warning; legacy_code: string | null }>} */ let warnings = []; +/** @type {Set[]} */ +let ignore_stack = []; + /** @type {string | undefined} */ let filename; @@ -15,33 +18,79 @@ let locator = getLocator('', { offsetLine: 1 }); * source: string; * filename: string | undefined; * }} options - * @returns {import('#compiler').Warning[]} */ export function reset_warnings(options) { filename = options.filename; + ignore_stack = []; + warnings = []; locator = getLocator(options.source, { offsetLine: 1 }); +} - return (warnings = []); +/** + * @param {boolean} is_runes + */ +export function get_warnings(is_runes) { + /** @type {import('#compiler').Warning[]} */ + const final = []; + for (const { warning, legacy_code } of warnings) { + if (legacy_code) { + if (is_runes) { + final.push({ + ...warning, + message: + (warning.message += ` (this warning was tried to silence using code ${legacy_code}. In runes mode, use the new code ${warning.code} instead)`) + }); + } + } else { + final.push(warning); + } + } + + return final; } +/** + * @param {string[]} ignores + */ +export function push_ignore(ignores) { + const next = new Set([...(ignore_stack.at(-1) || []), ...ignores]); + ignore_stack.push(next); +} + +export function pop_ignore() { + ignore_stack.pop(); +} + +// TODO add more mappings for prominent codes that have been renamed +const legacy_codes = new Map([ + ['reactive_declaration_invalid_placement', 'non-top-level-reactive-declaration'] +]); + /** * @param {null | NodeLike} node * @param {string} code * @param {string} message */ function w(node, code, message) { - // @ts-expect-error - if (node?.ignores?.has(code)) return; + const ignores = ignore_stack.at(-1); + if (ignores?.has(code)) return; + // backwards compat: Svelte 5 changed all warnings from dash to underscore - // @ts-expect-error - if (node?.ignores?.has(code.replaceAll('_', '-'))) return; + /** @type {string | null} */ + let legacy_code = legacy_codes.get(code) || code.replaceAll('_', '-'); + if (!ignores?.has(legacy_code)) { + legacy_code = null; + } warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined + legacy_code, + warning: { + code, + message, + filename, + start: node?.start !== undefined ? locator(node.start) : undefined, + end: node?.end !== undefined ? locator(node.end) : undefined + } }); } diff --git a/packages/svelte/src/compiler/index.js b/packages/svelte/src/compiler/index.js index ba312a1a639e..4a10b84d055a 100644 --- a/packages/svelte/src/compiler/index.js +++ b/packages/svelte/src/compiler/index.js @@ -8,7 +8,7 @@ import { remove_typescript_nodes } from './phases/1-parse/remove_typescript_node import { analyze_component, analyze_module } from './phases/2-analyze/index.js'; import { transform_component, transform_module } from './phases/3-transform/index.js'; import { validate_component_options, validate_module_options } from './validate-options.js'; -import { reset_warnings } from './warnings.js'; +import { get_warnings, reset_warnings } from './warnings.js'; export { default as preprocess } from './preprocess/index.js'; /** @@ -21,7 +21,7 @@ export { default as preprocess } from './preprocess/index.js'; */ export function compile(source, options) { try { - const warnings = reset_warnings({ source, filename: options.filename }); + reset_warnings({ source, filename: options.filename }); const validated = validate_component_options(options, ''); let parsed = _parse(source); @@ -46,7 +46,7 @@ export function compile(source, options) { const analysis = analyze_component(parsed, source, combined_options); const result = transform_component(analysis, source, combined_options); - result.warnings = warnings; + result.warnings = get_warnings(analysis.runes); result.ast = to_public_ast(source, parsed, options.modernAst); return result; } catch (e) { @@ -68,11 +68,11 @@ export function compile(source, options) { */ export function compileModule(source, options) { try { - const warnings = reset_warnings({ source, filename: options.filename }); + reset_warnings({ source, filename: options.filename }); const validated = validate_module_options(options, ''); const analysis = analyze_module(parse_acorn(source, false), validated); const result = transform_module(analysis, source, validated); - result.warnings = warnings; + result.warnings = get_warnings(true); return result; } catch (e) { if (e instanceof CompileError) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/a11y.js b/packages/svelte/src/compiler/phases/2-analyze/a11y.js index 5e6e05ab01be..a45c3c1f97ea 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/a11y.js +++ b/packages/svelte/src/compiler/phases/2-analyze/a11y.js @@ -702,12 +702,6 @@ function check_element(node, state) { let has_contenteditable_binding = false; for (const attribute of node.attributes) { - // gross but necessary: the visitor that sets this isn't called in time - if (state.ignores.size > 0) { - // @ts-expect-error - attribute.ignores = state.ignores; - } - if (attribute.type === 'SpreadAttribute') { has_spread = true; } else if (attribute.type === 'OnDirective') { diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index e4d6f6a4db9c..306c740f7b19 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -439,8 +439,7 @@ export function analyze_component(root, source, options) { component_slots: new Set(), expression: null, private_derived_state: [], - function_depth: scope.function_depth, - ignores: new Set() + function_depth: scope.function_depth }; walk( @@ -511,8 +510,7 @@ export function analyze_component(root, source, options) { component_slots: new Set(), expression: null, private_derived_state: [], - function_depth: scope.function_depth, - ignores: new Set() + function_depth: scope.function_depth }; walk( @@ -1093,62 +1091,44 @@ function is_safe_identifier(expression, scope) { /** @type {import('./types').Visitors} */ const common_visitors = { - _(node, context) { - // @ts-expect-error - const comments = /** @type {import('estree').Comment[]} */ (node.leadingComments); - - if (comments) { + _(node, { next, path }) { + const parent = path.at(-1); + if (parent?.type === 'Fragment') { + const idx = parent.nodes.indexOf(/** @type {any} */ (node)); /** @type {string[]} */ const ignores = []; - for (const comment of comments) { - ignores.push(...extract_svelte_ignore(comment.value)); + for (let i = idx - 1; i >= 0; i--) { + const prev = parent.nodes[i]; + if (prev.type === 'Comment') { + ignores.push(...extract_svelte_ignore(prev.data)); + } else if (prev.type !== 'Text') { + break; + } } if (ignores.length > 0) { - // @ts-expect-error see below - node.ignores = new Set([...context.state.ignores, ...ignores]); + w.push_ignore(ignores); + next(); + w.pop_ignore(); } - } - - // @ts-expect-error - if (node.ignores) { - context.next({ - ...context.state, - // @ts-expect-error see below - ignores: node.ignores - }); - } else if (context.state.ignores.size > 0) { + } else { // @ts-expect-error - node.ignores = context.state.ignores; - } - }, - Fragment(node, context) { - /** @type {string[]} */ - let ignores = []; + const comments = /** @type {import('estree').Comment[]} */ (node.leadingComments); - for (const child of node.nodes) { - if (child.type === 'Text' && child.data.trim() === '') { - continue; - } + if (comments) { + /** @type {string[]} */ + const ignores = []; - if (child.type === 'Comment') { - ignores.push(...extract_svelte_ignore(child.data)); - } else { - const combined_ignores = new Set(context.state.ignores); - for (const ignore of ignores) combined_ignores.add(ignore); - - if (combined_ignores.size > 0) { - // TODO this is a grotesque hack that's made necessary by the fact that - // we can't call `context.visit(...)` here, because we do the convoluted - // visitor merging thing. I'm increasingly of the view that we should - // rearchitect this stuff and have a single visitor per node. It'd be - // more efficient and much simpler. - // @ts-expect-error - child.ignores = combined_ignores; + for (const comment of comments) { + ignores.push(...extract_svelte_ignore(comment.value)); } - ignores = []; + if (ignores.length > 0) { + w.push_ignore(ignores); + next(); + w.pop_ignore(); + } } } }, diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index cc815fae7b7b..d2c503e8b108 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -22,7 +22,6 @@ export interface AnalysisState { expression: ExpressionTag | ClassDirective | SpreadAttribute | null; private_derived_state: string[]; function_depth: number; - ignores: Set; } export interface LegacyAnalysisState extends AnalysisState { diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 8cd855af854c..5d43f466dc62 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -3,8 +3,10 @@ import { getLocator } from 'locate-character'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -/** @type {import('#compiler').Warning[]} */ +/** @type {Array<{ warning: import('#compiler').Warning; legacy_code: string | null }>} */ let warnings = []; +/** @type {Set[]} */ +let ignore_stack = []; /** @type {string | undefined} */ let filename; let locator = getLocator('', { offsetLine: 1 }); @@ -14,32 +16,85 @@ let locator = getLocator('', { offsetLine: 1 }); * source: string; * filename: string | undefined; * }} options - * @returns {import('#compiler').Warning[]} */ export function reset_warnings(options) { filename = options.filename; + ignore_stack = []; + warnings = []; locator = getLocator(options.source, { offsetLine: 1 }); - return warnings = []; } +/** + * @param {boolean} is_runes + */ +export function get_warnings(is_runes) { + /** @type {import('#compiler').Warning[]} */ + const final = []; + + for (const { warning, legacy_code } of warnings) { + if (legacy_code) { + if (is_runes) { + final.push({ + ...warning, + message: warning.message += ` (this warning was tried to silence using code ${legacy_code}. In runes mode, use the new code ${warning.code} instead)` + }); + } + } else { + final.push(warning); + } + } + + return final; +} + +/** + * @param {string[]} ignores + */ +export function push_ignore(ignores) { + const next = new Set([...ignore_stack.at(-1) || [], ...ignores]); + + ignore_stack.push(next); +} + +export function pop_ignore() { + ignore_stack.pop(); +} + +// TODO add more mappings for prominent codes that have been renamed +const legacy_codes = new Map([ + [ + 'reactive_declaration_invalid_placement', + 'non-top-level-reactive-declaration' + ] +]); + /** * @param {null | NodeLike} node * @param {string} code * @param {string} message */ function w(node, code, message) { - // @ts-expect-error - if (node?.ignores?.has(code)) return; + const ignores = ignore_stack.at(-1); + + if (ignores?.has(code)) return; + // backwards compat: Svelte 5 changed all warnings from dash to underscore - // @ts-expect-error - if (node?.ignores?.has(code.replaceAll('_', '-'))) return; + /** @type {string | null} */ + let legacy_code = legacy_codes.get(code) || code.replaceAll('_', '-'); + + if (!ignores?.has(legacy_code)) { + legacy_code = null; + } warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined + legacy_code, + warning: { + code, + message, + filename, + start: node?.start !== undefined ? locator(node.start) : undefined, + end: node?.end !== undefined ? locator(node.end) : undefined + } }); } diff --git a/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte index 9a25e77ac13f..faae4636e5bf 100644 --- a/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte +++ b/packages/svelte/tests/validator/samples/ignore-warning-dash-backwards-compat/input.svelte @@ -1,6 +1,8 @@ diff --git a/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/input.svelte b/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/input.svelte new file mode 100644 index 000000000000..268bb2e867f5 --- /dev/null +++ b/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/input.svelte @@ -0,0 +1,9 @@ + + + +
+ +
+ + +
diff --git a/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/warnings.json b/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/warnings.json new file mode 100644 index 000000000000..8bb98aff0eb9 --- /dev/null +++ b/packages/svelte/tests/validator/samples/ignore-warning-dash-runes/warnings.json @@ -0,0 +1,26 @@ +[ + { + "code": "a11y_missing_attribute", + "end": { + "column": 29, + "line": 5 + }, + "message": "`` element should have an alt attribute (this warning was tried to silence using code a11y-missing-attribute. In runes mode, use the new code a11y_missing_attribute instead)", + "start": { + "column": 1, + "line": 5 + } + }, + { + "code": "a11y_misplaced_scope", + "end": { + "column": 10, + "line": 9 + }, + "message": "The scope attribute should only be used with `` elements (this warning was tried to silence using code a11y-misplaced-scope. In runes mode, use the new code a11y_misplaced_scope instead)", + "start": { + "column": 5, + "line": 9 + } + } +] From 0c05de40425e31fe84d76f88c6987ffefe5810ec Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 8 May 2024 21:23:03 +0200 Subject: [PATCH 6/7] take renames into account when migrating warnings --- .../templates/compile-warnings.js | 9 ++++++ packages/svelte/src/compiler/migrate/index.js | 30 ++++++++++++------- packages/svelte/src/compiler/warnings.js | 9 ++++++ .../samples/svelte-ignore/input.svelte | 6 ++-- .../samples/svelte-ignore/output.svelte | 6 ++-- 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index 3c9c8d13febe..97e0d860dcfe 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -66,6 +66,15 @@ const legacy_codes = new Map([ ['reactive_declaration_invalid_placement', 'non-top-level-reactive-declaration'] ]); +const new_codes = new Map([...legacy_codes.entries()].map(([key, value]) => [value, key])); + +/** + * @param {string} legacy_code + */ +export function legacy_to_new_code(legacy_code) { + return new_codes.get(legacy_code) || legacy_code.replaceAll('-', '_'); +} + /** * @param {null | NodeLike} node * @param {string} code diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 12e5085dbb22..b00a9b8343b6 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -4,7 +4,7 @@ import { parse } from '../phases/1-parse/index.js'; import { analyze_component } from '../phases/2-analyze/index.js'; import { validate_component_options } from '../validate-options.js'; import { get_rune } from '../phases/scope.js'; -import { reset_warnings } from '../warnings.js'; +import { legacy_to_new_code, reset_warnings } from '../warnings.js'; import { extract_identifiers } from '../utils/ast.js'; import { regex_is_valid_identifier } from '../phases/patterns.js'; import { extract_svelte_ignore } from '../utils/extract_svelte_ignore.js'; @@ -180,13 +180,20 @@ const instance_script = { const comments = node.leadingComments; if (comments) { for (const comment of comments) { - if (comment.type === 'Line' && extract_svelte_ignore(comment.value).length > 0) { - const svelte_ignore = comment.value.indexOf('svelte-ignore'); - state.str.overwrite( - comment.start + svelte_ignore + 13 + '//'.length, - comment.end, - comment.value.substring(svelte_ignore + 13).replaceAll('-', '_') - ); + if (comment.type === 'Line') { + /** @type {string} */ + const text = comment.value; + const ignores = extract_svelte_ignore(text); + if (ignores.length > 0) { + const svelte_ignore = text.indexOf('svelte-ignore'); + state.str.overwrite( + comment.start + svelte_ignore + 13 + '//'.length, + comment.end, + text + .substring(svelte_ignore + 13) + .replace(new RegExp(ignores.join('|'), 'g'), (match) => legacy_to_new_code(match)) + ); + } } } } @@ -494,12 +501,15 @@ const template = { } }, Comment(node, { state }) { - if (extract_svelte_ignore(node.data).length > 0) { + const ignores = extract_svelte_ignore(node.data); + if (ignores.length > 0) { const svelte_ignore = node.data.indexOf('svelte-ignore'); state.str.overwrite( node.start + svelte_ignore + 13 + ''.length, - node.data.substring(svelte_ignore + 13).replaceAll('-', '_') + node.data + .substring(svelte_ignore + 13) + .replace(new RegExp(ignores.join('|'), 'g'), (match) => legacy_to_new_code(match)) ); } } diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 5d43f466dc62..52e50833636b 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -68,6 +68,15 @@ const legacy_codes = new Map([ ] ]); +const new_codes = new Map([...legacy_codes.entries()].map(([key, value]) => [value, key])); + +/** + * @param {string} legacy_code + */ +export function legacy_to_new_code(legacy_code) { + return new_codes.get(legacy_code) || legacy_code.replaceAll('-', '_'); +} + /** * @param {null | NodeLike} node * @param {string} code diff --git a/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte b/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte index b065e7c49802..fa734942f9ff 100644 --- a/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte +++ b/packages/svelte/tests/migrate/samples/svelte-ignore/input.svelte @@ -1,6 +1,8 @@ diff --git a/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte b/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte index 42c62798d2b2..fa305cefb1fa 100644 --- a/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte +++ b/packages/svelte/tests/migrate/samples/svelte-ignore/output.svelte @@ -1,6 +1,8 @@ From 546cc7d6f0cdeb3bd1d0e6fee7ac825f86fd6d37 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 10 May 2024 17:42:21 -0400 Subject: [PATCH 7/7] move this out into separate PR (#11548) --- .../content/03-appendix/02-breaking-changes.md | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md index 8c4a2e0894b8..d5883b06aeb5 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md @@ -133,23 +133,6 @@ In Svelte 4 syntax, every property (declared via `export let`) is bindable, mean Setting the `accessors` option to `true` makes properties of a component directly accessible on the component instance. In runes mode, properties are never accessible on the component instance. You can use component exports instead if you need to expose them. -### Props may update with same value - -Svelte 4 did safe equality checks on properties by default. That meant that even if you update a value, and part of of tha value is a primitive passed as a property, and that part did not change, it would not trigger a property update. - -```svelte - - - -``` - -In Svelte 5, the above would trigger an update in the child component. Note that Svelte 4 by default did always update properties if they were not primitives (i.e. objects or arrays for example). - ## Other breaking changes ### Stricter `@const` assignment validation