diff --git a/.changeset/cold-birds-own.md b/.changeset/cold-birds-own.md new file mode 100644 index 0000000000..043f64b028 --- /dev/null +++ b/.changeset/cold-birds-own.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: add inline new class warning diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 5e8ba056ba..c0c8906acc 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -201,16 +201,20 @@ export function analyze_module(ast, options) { } } + /** @type {import('../types').RawWarning[]} */ + const warnings = []; + + const analysis = { + warnings + }; + walk( /** @type {import('estree').Node} */ (ast), - { scope }, + { scope, analysis }, // @ts-expect-error TODO clean this mess up merge(set_scope(scopes), validation_runes_js, runes_scope_js_tweaker) ); - /** @type {import('../types').RawWarning[]} */ - const warnings = []; - // If we are in runes mode, then check for possible misuses of state runes for (const [, scope] of scopes) { for (const [name, binding] of scope.declarations) { @@ -608,7 +612,7 @@ const legacy_scope_tweaker = { } }; -/** @type {import('zimmerframe').Visitors} */ +/** @type {import('zimmerframe').Visitors} */ const runes_scope_js_tweaker = { VariableDeclarator(node, { state }) { if (node.init?.type !== 'CallExpression') return; diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 2cf0129c53..8f8b8670b5 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -579,6 +579,26 @@ export const validation_runes_js = { ...context.state, private_derived_state }); + }, + NewExpression(node, context) { + const callee = node.callee; + + const binding = callee.type === 'Identifier' ? context.state.scope.get(callee.name) : null; + const is_module = context.state.ast_type === 'module'; + // In modules, we allow top-level module scope only, in components, we allow the component scope, + // which is function_depth of 1. With the exception of `new class` which is also not allowed at + // component scope level either. + const allowed_depth = is_module ? 0 : 1; + + if ( + (callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) || + (binding !== null && + binding.initial !== null && + binding.initial.type === 'ClassDeclaration' && + binding.scope.function_depth > allowed_depth) + ) { + warn(context.state.analysis.warnings, node, context.path, 'inline-new-class'); + } } }; @@ -721,5 +741,6 @@ export const validation_runes = merge(validation, a11y_validators, { } } }, - ClassBody: validation_runes_js.ClassBody + ClassBody: validation_runes_js.ClassBody, + NewExpression: validation_runes_js.NewExpression }); diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 22e5d3eaec..8d7cb526d4 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -437,7 +437,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { SwitchStatement: create_block_scope, ClassDeclaration(node, { state, next }) { - if (node.id) state.scope.declare(node.id, 'normal', 'const'); + if (node.id) state.scope.declare(node.id, 'normal', 'const', node); next(); }, diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 0d0f0dd487..f0981281fe 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -191,12 +191,20 @@ const state = { `State referenced in its own scope will never update. Did you mean to reference it inside a closure?` }; +/** @satisfies {Warnings} */ +const performance = { + 'inline-new-class': () => + `Creating inline classes will likely cause performance issues. ` + + `Instead, declare the class at the module-level and create new instances from the class reference.` +}; + /** @satisfies {Warnings} */ const warnings = { ...css, ...attributes, ...runes, ...a11y, + ...performance, ...state }; diff --git a/packages/svelte/tests/validator/samples/inline-new-class-2/input.svelte b/packages/svelte/tests/validator/samples/inline-new-class-2/input.svelte new file mode 100644 index 0000000000..34d31cd74b --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-2/input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/svelte/tests/validator/samples/inline-new-class-2/warnings.json b/packages/svelte/tests/validator/samples/inline-new-class-2/warnings.json new file mode 100644 index 0000000000..6ae8014ab5 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-2/warnings.json @@ -0,0 +1,14 @@ +[ + { + "code": "inline-new-class", + "message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.", + "start": { + "column": 12, + "line": 6 + }, + "end": { + "column": 21, + "line": 6 + } + } +] diff --git a/packages/svelte/tests/validator/samples/inline-new-class-3/input.svelte b/packages/svelte/tests/validator/samples/inline-new-class-3/input.svelte new file mode 100644 index 0000000000..1dd632df0e --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-3/input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/svelte/tests/validator/samples/inline-new-class-3/warnings.json b/packages/svelte/tests/validator/samples/inline-new-class-3/warnings.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-3/warnings.json @@ -0,0 +1 @@ +[] diff --git a/packages/svelte/tests/validator/samples/inline-new-class-4/input.svelte b/packages/svelte/tests/validator/samples/inline-new-class-4/input.svelte new file mode 100644 index 0000000000..1f8a6f5ba5 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-4/input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/svelte/tests/validator/samples/inline-new-class-4/warnings.json b/packages/svelte/tests/validator/samples/inline-new-class-4/warnings.json new file mode 100644 index 0000000000..7a3637a4a3 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-4/warnings.json @@ -0,0 +1,14 @@ +[ + { + "code": "inline-new-class", + "message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.", + "start": { + "column": 12, + "line": 3 + }, + "end": { + "column": 3, + "line": 5 + } + } +] diff --git a/packages/svelte/tests/validator/samples/inline-new-class-5/input.svelte b/packages/svelte/tests/validator/samples/inline-new-class-5/input.svelte new file mode 100644 index 0000000000..817bb29310 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-5/input.svelte @@ -0,0 +1,5 @@ + diff --git a/packages/svelte/tests/validator/samples/inline-new-class-5/warnings.json b/packages/svelte/tests/validator/samples/inline-new-class-5/warnings.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class-5/warnings.json @@ -0,0 +1 @@ +[] diff --git a/packages/svelte/tests/validator/samples/inline-new-class/input.svelte b/packages/svelte/tests/validator/samples/inline-new-class/input.svelte new file mode 100644 index 0000000000..10a29724ff --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class/input.svelte @@ -0,0 +1,5 @@ + diff --git a/packages/svelte/tests/validator/samples/inline-new-class/warnings.json b/packages/svelte/tests/validator/samples/inline-new-class/warnings.json new file mode 100644 index 0000000000..d57fc6ca15 --- /dev/null +++ b/packages/svelte/tests/validator/samples/inline-new-class/warnings.json @@ -0,0 +1,14 @@ +[ + { + "code": "inline-new-class", + "message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.", + "start": { + "column": 11, + "line": 2 + }, + "end": { + "column": 2, + "line": 4 + } + } +]