From 2bc39b1de2d160368c2ad8663c8876efb9736ff0 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 14 May 2024 11:24:43 +0200 Subject: [PATCH] chore: better ignore code handling (#11606) Instead of hacking an ignores array onto each node (and possibly degrading perf a bit because the object shape is mutated) we keep track of ignores in a stack. The new approach also avoids the indirection the old one had to do because the new approach looks upwards (checking if parent is a fragment) instead of iterating the children (checking for comments in them). As a bonus unknown code warnings are now in order (line-column-wise) with the other warnings. Also fixes #11482 because text nodes of all shapes are ok --- .../templates/compile-warnings.js | 5 +- .../src/compiler/phases/2-analyze/a11y.js | 3 - .../src/compiler/phases/2-analyze/index.js | 99 ++++++++----------- .../src/compiler/phases/2-analyze/types.d.ts | 1 - packages/svelte/src/compiler/state.js | 16 +++ packages/svelte/src/compiler/warnings.js | 5 +- .../samples/unknown-code/warnings.json | 84 ++++++++-------- 7 files changed, 103 insertions(+), 110 deletions(-) diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index 75d0e3603f..15908fcec2 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -1,4 +1,4 @@ -import { filename, locator, warnings } from './state.js'; +import { filename, locator, warnings, ignore_stack } from './state.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ @@ -8,8 +8,7 @@ import { filename, locator, warnings } from './state.js'; * @param {string} message */ function w(node, code, message) { - // @ts-expect-error - if (node?.ignores?.has(code)) return; + if (ignore_stack.at(-1)?.has(code)) 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 f40fc7c12e..a45c3c1f97 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/a11y.js +++ b/packages/svelte/src/compiler/phases/2-analyze/a11y.js @@ -727,9 +727,6 @@ function check_element(node, state) { for (const attribute of node.attributes) { if (attribute.type !== 'Attribute') continue; - // @ts-expect-error gross hack - attribute.ignores = node.ignores; - const name = attribute.name.toLowerCase(); // aria-props if (name.startsWith('aria-')) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 041f8d8dd2..55373baa41 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -25,6 +25,7 @@ import { prune } from './css/css-prune.js'; import { hash } from './utils.js'; import { warn_unused } from './css/css-warn.js'; import { extract_svelte_ignore } from '../../utils/extract_svelte_ignore.js'; +import { pop_ignore, push_ignore } from '../../state.js'; /** * @param {import('#compiler').Script | null} script @@ -439,8 +440,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 +511,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( @@ -1100,67 +1099,51 @@ 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, { state, next, path }) { + const parent = path.at(-1); + if (parent?.type === 'Fragment' && node.type !== 'Comment' && node.type !== 'Text') { + const idx = parent.nodes.indexOf(/** @type {any} */ (node)); /** @type {string[]} */ const ignores = []; - - for (const comment of comments) { - const start = /** @type {any} */ (comment).start + 2; - ignores.push(...extract_svelte_ignore(start, comment.value, context.state.analysis.runes)); + for (let i = idx - 1; i >= 0; i--) { + const prev = parent.nodes[i]; + if (prev.type === 'Comment') { + ignores.push( + ...extract_svelte_ignore( + prev.start + 2 /* '//'.length */, + prev.data, + state.analysis.runes + ) + ); + } else if (prev.type !== 'Text') { + break; + } } if (ignores.length > 0) { - // @ts-expect-error see below - node.ignores = new Set([...context.state.ignores, ...ignores]); + push_ignore(ignores); + next(); + 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) { - // @ts-expect-error - node.ignores = context.state.ignores; - } - }, - Fragment(node, context) { - /** @type {string[]} */ - let ignores = []; - - for (const child of node.nodes) { - if (child.type === 'Text' && child.data.trim() === '') { - continue; - } - - if (child.type === 'Comment') { - const start = - child.start + - (context.state.analysis.source.slice(child.start, child.start + 4) === '