chore: add inline new class warning (#9583)

* chore: add inline new class warning

* Address feedback

* address feedback

* more tests

Author: trueadm

.changeset/cold-birds-own.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: add inline new class warning

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<import('#compiler').SvelteNode, { scope: Scope }>} */
+/** @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, { scope: Scope, analysis: { warnings: import('../types').RawWarning[] } }>} */
 const runes_scope_js_tweaker = {
 	VariableDeclarator(node, { state }) {
 		if (node.init?.type !== 'CallExpression') return;

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
 });

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();
 		},
 

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
 };
 

packages/svelte/tests/validator/samples/inline-new-class-2/input.svelte

@@ -0,0 +1,8 @@
+<script>
+	function bar() {
+		class Foo {
+			foo = $state(0)
+		}
+		const a = new Foo();
+	}
+</script>

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
+		}
+	}
+]

packages/svelte/tests/validator/samples/inline-new-class-3/input.svelte

@@ -0,0 +1,8 @@
+<script>
+	class Foo {
+		foo = $state(0)
+	}
+	function bar() {
+		const a = new Foo();
+	}
+</script>

packages/svelte/tests/validator/samples/inline-new-class-3/warnings.json

@@ -0,0 +1 @@
+[]

packages/svelte/tests/validator/samples/inline-new-class-4/input.svelte

@@ -0,0 +1,7 @@
+<script>
+	function bar() {
+		const a = new class Foo {
+			foo = $state(0)
+		}
+	}
+</script>

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
+		}
+	}
+]

packages/svelte/tests/validator/samples/inline-new-class-5/input.svelte

@@ -0,0 +1,5 @@
+<script context="module">
+	const a = new class Foo {
+		foo = $state(0)
+	}
+</script>

packages/svelte/tests/validator/samples/inline-new-class-5/warnings.json

@@ -0,0 +1 @@
+[]

packages/svelte/tests/validator/samples/inline-new-class/input.svelte

@@ -0,0 +1,5 @@
+<script>
+	const a = new class {
+		foo = $state(0)
+	}
+</script>

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
+		}
+	}
+]