msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

chore: refactor logic for ignoring warnings (#11300)

Browse Source 
* reimplement svelte-ignore logic

* simplify

* remove unused code

* remove ignores array from comment node

* unused

* regenerate types

* types

* oops, again
pull/11303/head
Rich Harris 8 months ago committed by GitHub
parent 73490bbb8e
commit 5405ec6c91
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 132 additions and 143 deletions
Show Stats Download Patch File Download Diff File

9
packages/svelte/src/compiler/legacy.js
Unescape View File

@ -4,6 +4,7 @@ import {
regex_not_whitespace,
regex_starts_with_whitespaces
} from './phases/patterns.js';
import { extract_svelte_ignore } from './utils/extract_svelte_ignore.js';
/**
* Some of the legacy Svelte AST nodes remove whitespace from the start and end of their children.
@ -197,7 +198,13 @@ export function convert(source, ast) {
ClassDirective(node) {
return { ...node, type: 'Class' };
},
ComplexSelector(node, { visit }) {
Comment(node) {
return {
...node,
ignores: extract_svelte_ignore(node.data)
};
},
ComplexSelector(node) {
const children = [];
for (const child of node.children) {

5
packages/svelte/src/compiler/phases/1-parse/state/element.js
Unescape View File

@ -1,5 +1,3 @@
import { extract_svelte_ignore } from '../../../utils/extract_svelte_ignore.js';
import fuzzymatch from '../utils/fuzzymatch.js';
import { is_void } from '../utils/names.js';
import read_expression from '../read/expression.js';
import { read_script } from '../read/script.js';
@ -87,8 +85,7 @@ export default function tag(parser) {
type: 'Comment',
start,
end: parser.index,
data,
ignores: extract_svelte_ignore(data)
data
});
return;

10
packages/svelte/src/compiler/phases/2-analyze/a11y.js
Unescape View File

@ -667,9 +667,8 @@ function get_static_text_value(attribute) {
/**
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} node
* @param {import('./types.js').AnalysisState} state
* @param {import('#compiler').SvelteNode[]} path
*/
function check_element(node, state, path) {
function check_element(node, state) {
// foreign namespace means elements can have completely different meanings, therefore we don't check them
if (state.options.namespace === 'foreign') return;
@ -680,8 +679,7 @@ function check_element(node, state, path) {
* @param {Parameters<import('../../warnings.js').AllWarnings[T]>} args
* @returns {void}
*/
const push_warning = (node, code, ...args) =>
warn(state.analysis.warnings, node, path, code, ...args);
const push_warning = (node, code, ...args) => warn(state.analysis.warnings, node, code, ...args);
/** @type {Map<string, import('#compiler').Attribute>} */
const attribute_map = new Map();
@ -1165,9 +1163,9 @@ function check_element(node, state, path) {
*/
export const a11y_validators = {
RegularElement(node, context) {
check_element(node, context.state, context.path);
check_element(node, context.state);
},
SvelteElement(node, context) {
check_element(node, context.state, context.path);
check_element(node, context.state);
}
};

2
packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js
Unescape View File

@ -26,7 +26,7 @@ const visitors = {
if (!node.metadata.used) {
const content = context.state.stylesheet.content;
const text = content.styles.substring(node.start - content.start, node.end - content.start);
warn(context.state.warnings, node, context.path, 'css-unused-selector', text);
warn(context.state.warnings, node, 'css-unused-selector', text);
}
context.next();

86
packages/svelte/src/compiler/phases/2-analyze/index.js
Unescape View File

@ -24,6 +24,7 @@ import { analyze_css } from './css/css-analyze.js';
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';
/**
* @param {import('#compiler').Script | null} script
@ -330,7 +331,7 @@ export function analyze_component(root, source, options) {
} else if (declaration !== null && Runes.includes(/** @type {any} */ (name))) {
for (const { node, path } of references) {
if (path.at(-1)?.type === 'CallExpression') {
warn(warnings, node, [], 'store-with-rune-name', store_name);
warn(warnings, node, 'store-with-rune-name', store_name);
}
}
}
@ -407,7 +408,7 @@ export function analyze_component(root, source, options) {
};
if (!options.customElement && root.options?.customElement) {
warn(analysis.warnings, root.options, [], 'missing-custom-element-compile-option');
warn(analysis.warnings, root.options, 'missing-custom-element-compile-option');
}
if (analysis.runes) {
@ -433,7 +434,8 @@ export function analyze_component(root, source, options) {
component_slots: new Set(),
expression: null,
private_derived_state: [],
function_depth: scope.function_depth
function_depth: scope.function_depth,
ignores: new Set()
};
walk(
@ -475,7 +477,8 @@ export function analyze_component(root, source, options) {
component_slots: new Set(),
expression: null,
private_derived_state: [],
function_depth: scope.function_depth
function_depth: scope.function_depth,
ignores: new Set()
};
walk(
@ -495,7 +498,7 @@ export function analyze_component(root, source, options) {
(r) => r.node !== binding.node && r.path.at(-1)?.type !== 'ExportSpecifier'
);
if (!references.length && !instance.scope.declarations.has(`$${name}`)) {
warn(warnings, binding.node, [], 'unused-export-let', name);
warn(warnings, binding.node, 'unused-export-let', name);
}
}
}
@ -535,7 +538,7 @@ export function analyze_component(root, source, options) {
type === 'AwaitBlock' ||
type === 'KeyBlock'
) {
warn(warnings, binding.node, [], 'non-state-reference', name);
warn(warnings, binding.node, 'non-state-reference', name);
continue outer;
}
}
@ -543,7 +546,7 @@ export function analyze_component(root, source, options) {
}
}
warn(warnings, binding.node, [], 'non-state-reference', name);
warn(warnings, binding.node, 'non-state-reference', name);
continue outer;
}
}
@ -557,7 +560,13 @@ export function analyze_component(root, source, options) {
for (const element of analysis.elements) {
prune(analysis.css.ast, element);
}
if (
!analysis.css.ast.content.comment ||
!extract_svelte_ignore(analysis.css.ast.content.comment.data).includes('css-unused-selector')
) {
warn_unused(analysis.css.ast, analysis.warnings);
}
outer: for (const element of analysis.elements) {
if (element.metadata.scoped) {
@ -679,7 +688,7 @@ const legacy_scope_tweaker = {
(d) => d.scope === state.analysis.module.scope && d.declaration_kind !== 'const'
)
) {
warn(state.analysis.warnings, node, path, 'module-script-reactive-declaration');
warn(state.analysis.warnings, node, 'module-script-reactive-declaration');
}
if (
@ -1045,6 +1054,65 @@ const function_visitor = (node, context) => {
/** @type {import('./types').Visitors} */
const common_visitors = {
_(node, context) {
// @ts-expect-error
const comments = /** @type {import('estree').Comment[]} */ (node.leadingComments);
if (comments) {
/** @type {string[]} */
const ignores = [];
for (const comment of comments) {
ignores.push(...extract_svelte_ignore(comment.value));
}
if (ignores.length > 0) {
// @ts-expect-error see below
node.ignores = new Set([...context.state.ignores, ...ignores]);
}
}
// @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') {
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;
}
ignores = [];
}
}
},
Attribute(node, context) {
if (node.value === true) return;
@ -1142,7 +1210,7 @@ const common_visitors = {
binding.kind === 'derived') &&
context.state.function_depth === binding.scope.function_depth
) {
warn(context.state.analysis.warnings, node, context.path, 'static-state-reference');
warn(context.state.analysis.warnings, node, 'static-state-reference');
}
}
},

1
packages/svelte/src/compiler/phases/2-analyze/types.d.ts vendored
Unescape View File

@ -22,6 +22,7 @@ export interface AnalysisState {
expression: ExpressionTag | ClassDirective | SpreadAttribute | null;
private_derived_state: string[];
function_depth: number;
ignores: Set<string>;
}
export interface LegacyAnalysisState extends AnalysisState {

47
packages/svelte/src/compiler/phases/2-analyze/validation.js
Unescape View File

@ -134,7 +134,6 @@ function validate_element(node, context) {
warn(
context.state.analysis.warnings,
attribute,
context.path,
'global-event-reference',
attribute.name
);
@ -147,7 +146,7 @@ function validate_element(node, context) {
}
if (attribute.name === 'is' && context.state.options.namespace !== 'foreign') {
warn(context.state.analysis.warnings, attribute, context.path, 'avoid-is');
warn(context.state.analysis.warnings, attribute, 'avoid-is');
}
const correct_name = react_attributes.get(attribute.name);
@ -155,7 +154,6 @@ function validate_element(node, context) {
warn(
context.state.analysis.warnings,
attribute,
context.path,
'invalid-html-attribute',
attribute.name,
correct_name
@ -235,7 +233,7 @@ function validate_attribute_name(attribute, context) {
!attribute.name.startsWith('xlink:') &&
!attribute.name.startsWith('xml:')
) {
warn(context.state.analysis.warnings, attribute, context.path, 'illegal-attribute-character');
warn(context.state.analysis.warnings, attribute, 'illegal-attribute-character');
}
}
@ -313,7 +311,7 @@ function validate_block_not_empty(node, context) {
// Assumption: If the block has zero elements, someone's in the middle of typing it out,
// so don't warn in that case because it would be distracting.
if (node.nodes.length === 1 && node.nodes[0].type === 'Text' && !node.nodes[0].raw.trim()) {
warn(context.state.analysis.warnings, node.nodes[0], context.path, 'empty-block');
warn(context.state.analysis.warnings, node.nodes[0], 'empty-block');
}
}
@ -377,7 +375,6 @@ const validation = {
warn(
context.state.analysis.warnings,
binding.node,
context.path,
'invalid-rest-eachblock-binding',
binding.node.name
);
@ -534,13 +531,7 @@ const validation = {
binding.declaration_kind === 'import' &&
binding.references.length === 0
) {
warn(
context.state.analysis.warnings,
node,
context.path,
'component-name-lowercase',
node.name
);
warn(context.state.analysis.warnings, node, 'component-name-lowercase', node.name);
}
validate_element(node, context);
@ -579,13 +570,7 @@ const validation = {
!VoidElements.includes(node.name) &&
!SVGElements.includes(node.name)
) {
warn(
context.state.analysis.warnings,
node,
context.path,
'invalid-self-closing-tag',
node.name
);
warn(context.state.analysis.warnings, node, 'invalid-self-closing-tag', node.name);
}
context.next({
@ -781,7 +766,7 @@ export const validation_legacy = merge(validation, a11y_validators, {
(state.ast_type !== 'instance' ||
/** @type {import('#compiler').SvelteNode} */ (path.at(-1)).type !== 'Program')
) {
warn(state.analysis.warnings, node, path, 'no-reactive-declaration');
warn(state.analysis.warnings, node, 'no-reactive-declaration');
}
},
UpdateExpression(node, { state }) {
@ -984,12 +969,12 @@ export const validation_runes_js = {
const allowed_depth = context.state.ast_type === 'module' ? 0 : 1;
if (context.state.scope.function_depth > allowed_depth) {
warn(context.state.analysis.warnings, node, context.path, 'avoid-nested-class');
warn(context.state.analysis.warnings, node, 'avoid-nested-class');
}
},
NewExpression(node, context) {
if (node.callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) {
warn(context.state.analysis.warnings, node, context.path, 'avoid-inline-class');
warn(context.state.analysis.warnings, node, 'avoid-inline-class');
}
}
};
@ -1134,7 +1119,7 @@ export const validation_runes = merge(validation, a11y_validators, {
}
next({ ...state });
},
VariableDeclarator(node, { state, path }) {
VariableDeclarator(node, { state }) {
ensure_no_module_import_conflict(node, state);
const init = node.init;
@ -1142,7 +1127,7 @@ export const validation_runes = merge(validation, a11y_validators, {
if (rune === null) {
if (init?.type === 'Identifier' && init.name === '$props' && !state.scope.get('props')) {
warn(state.analysis.warnings, node, path, 'invalid-props-declaration');
warn(state.analysis.warnings, node, 'invalid-props-declaration');
}
return;
}
@ -1195,29 +1180,29 @@ export const validation_runes = merge(validation, a11y_validators, {
arg.type === 'CallExpression' &&
(arg.callee.type === 'ArrowFunctionExpression' || arg.callee.type === 'FunctionExpression')
) {
warn(state.analysis.warnings, node, path, 'derived-iife');
warn(state.analysis.warnings, node, 'derived-iife');
}
}
},
AssignmentPattern(node, { state, path }) {
AssignmentPattern(node, { state }) {
if (
node.right.type === 'Identifier' &&
node.right.name === '$bindable' &&
!state.scope.get('bindable')
) {
warn(state.analysis.warnings, node, path, 'invalid-bindable-declaration');
warn(state.analysis.warnings, node, 'invalid-bindable-declaration');
}
},
SlotElement(node, { state, path }) {
SlotElement(node, { state }) {
if (!state.analysis.custom_element) {
warn(state.analysis.warnings, node, path, 'deprecated-slot-element');
warn(state.analysis.warnings, node, 'deprecated-slot-element');
}
},
OnDirective(node, { state, path }) {
const parent_type = path.at(-1)?.type;
// Don't warn on component events; these might not be under the author's control so the warning would be unactionable
if (parent_type === 'RegularElement' || parent_type === 'SvelteElement') {
warn(state.analysis.warnings, node, path, 'deprecated-event-handler', node.name);
warn(state.analysis.warnings, node, 'deprecated-event-handler', node.name);
}
},
// TODO this is a code smell. need to refactor this stuff

9
packages/svelte/src/compiler/types/legacy-nodes.d.ts vendored
Unescape View File

@ -198,6 +198,14 @@ export interface LegacyWindow extends BaseElement {
type: 'Window';
}
export interface LegacyComment extends BaseNode {
type: 'Comment';
/** the contents of the comment */
data: string;
/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
ignores: string[];
}
type LegacyDirective =
| LegacyAnimation
| LegacyBinding
@ -213,6 +221,7 @@ export type LegacyAttributeLike = LegacyAttribute | LegacySpread | LegacyDirecti
export type LegacyElementLike =
| LegacyBody
| LegacyCatchBlock
| LegacyComment
| LegacyDocument
| LegacyElement
| LegacyHead

2
packages/svelte/src/compiler/types/template.d.ts vendored
Unescape View File

@ -131,8 +131,6 @@ export interface Comment extends BaseNode {
type: 'Comment';
/** the contents of the comment */
data: string;
/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
ignores: string[];
}
/** A `{@const ...}` tag */

38
packages/svelte/src/compiler/utils/extract_svelte_ignore.js
Unescape View File

@ -15,41 +15,3 @@ export function extract_svelte_ignore(text) {
.filter(Boolean)
: [];
}
/**
* @template {{ leadingComments?: Array<{ value: string }> }} Node
* @param {Node} node
* @returns {string[]}
*/
export function extract_svelte_ignore_from_comments(node) {
return (node.leadingComments || []).flatMap(
/** @param {any} comment */ (comment) => extract_svelte_ignore(comment.value)
);
}
/**
* @param {import('#compiler').TemplateNode} node
* @param {import('#compiler').TemplateNode[]} template_nodes
* @returns {string[]}
*/
export function extract_ignores_above_position(node, template_nodes) {
const previous_node_idx = template_nodes.indexOf(node) - 1;
if (previous_node_idx < 0) {
return [];
}
const ignores = [];
for (let i = previous_node_idx; i >= 0; i--) {
const node = template_nodes[i];
if (node.type !== 'Comment' && node.type !== 'Text') {
return ignores;
}
if (node.type === 'Comment') {
if (node.ignores.length) {
ignores.push(...node.ignores);
}
}
}
return ignores;
}

50
packages/svelte/src/compiler/warnings.js
Unescape View File

@ -1,8 +1,3 @@
import {
extract_ignores_above_position,
extract_svelte_ignore_from_comments
} from './utils/extract_svelte_ignore.js';
/** @typedef {Record<string, (...args: any[]) => string>} Warnings */
/** @satisfies {Warnings} */
@ -277,52 +272,15 @@ const warnings = {
* @template {keyof AllWarnings} T
* @param {import('./phases/types').RawWarning[]} array the array to push the warning to, if not ignored
* @param {{ start?: number, end?: number, type?: string, parent?: import('#compiler').SvelteNode | null, leadingComments?: import('estree').Comment[] } | null} node the node related to the warning
* @param {import('#compiler').SvelteNode[]} path the path to the node, so that we can traverse upwards to find svelte-ignore comments
* @param {T} code the warning code
* @param {Parameters<AllWarnings[T]>} args the arguments to pass to the warning function
* @returns {void}
*/
export function warn(array, node, path, code, ...args) {
const fn = warnings[code];
// Traverse the AST upwards to find any svelte-ignore comments.
// This assumes that people don't have their code littered with warnings,
// at which point this might become inefficient.
/** @type {string[]} */
const ignores = [];
if (node) {
// comments inside JavaScript (estree)
if ('leadingComments' in node) {
ignores.push(...extract_svelte_ignore_from_comments(node));
}
}
for (let i = path.length - 1; i >= 0; i--) {
const current = path[i];
// comments inside JavaScript (estree)
if ('leadingComments' in current) {
ignores.push(...extract_svelte_ignore_from_comments(current));
}
// Svelte nodes
if (current.type === 'Fragment') {
ignores.push(
...extract_ignores_above_position(
/** @type {import('#compiler').TemplateNode} */ (path[i + 1] ?? node),
current.nodes
)
);
}
// Style nodes
if (current.type === 'StyleSheet' && current.content.comment) {
ignores.push(...current.content.comment.ignores);
}
}
export function warn(array, node, code, ...args) {
// @ts-expect-error
if (node.ignores?.has(code)) return;
if (ignores.includes(code)) return;
const fn = warnings[code];
const start = node?.start;
const end = node?.end;

3
packages/svelte/tests/parser-modern/samples/comment-before-script/output.json
Unescape View File

@ -11,8 +11,7 @@
"type": "Comment",
"start": 0,
"end": 27,
"data": "should not error out",
"ignores": []
"data": "should not error out"
},
{
"type": "Text",

11
packages/svelte/types/index.d.ts vendored
Unescape View File

@ -955,6 +955,14 @@ declare module 'svelte/compiler' {
type: 'Window';
}
interface LegacyComment extends BaseNode_1 {
type: 'Comment';
/** the contents of the comment */
data: string;
/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
ignores: string[];
}
type LegacyDirective =
| LegacyAnimation
| LegacyBinding
@ -970,6 +978,7 @@ declare module 'svelte/compiler' {
type LegacyElementLike =
| LegacyBody
| LegacyCatchBlock
| LegacyComment
| LegacyDocument
| LegacyElement
| LegacyHead
@ -1355,8 +1364,6 @@ declare module 'svelte/compiler' {
type: 'Comment';
/** the contents of the comment */
data: string;
/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
ignores: string[];
}
/** A `{@const ...}` tag */

Write Preview
Loading…
Cancel
Save