fix: allow children in valid-prop-names-in-kit-pages rule (#1048)

sveltejs · Feb 3, 2025 · 513806c · 513806c
1 parent f79fded
commit 513806c
Show file tree

Hide file tree

Showing 11 changed files with 183 additions and 35 deletions.
diff --git a/.changeset/gold-bottles-learn.md b/.changeset/gold-bottles-learn.md
@@ -0,0 +1,5 @@
+---
+'eslint-plugin-svelte': patch
+---
+
+fix: allow `children` in `valid-prop-names-in-kit-pages` rule
diff --git a/packages/eslint-plugin-svelte/src/rules/valid-prop-names-in-kit-pages.ts b/packages/eslint-plugin-svelte/src/rules/valid-prop-names-in-kit-pages.ts
@@ -2,16 +2,23 @@ import type { AST } from 'svelte-eslint-parser';
 import type { TSESTree } from '@typescript-eslint/types';
 import { createRule } from '../utils/index.js';
 import type { RuleContext } from '../types.js';
+import { getSvelteVersion } from '../utils/svelte-context.js';
+import { getFilename } from '../utils/compat.js';
 
 const EXPECTED_PROP_NAMES = ['data', 'errors', 'form', 'snapshot'];
+const EXPECTED_PROP_NAMES_SVELTE5 = [...EXPECTED_PROP_NAMES, 'children'];
 
-function checkProp(node: TSESTree.VariableDeclarator, context: RuleContext) {
+function checkProp(
+	node: TSESTree.VariableDeclarator,
+	context: RuleContext,
+	expectedPropNames: string[]
+) {
 	if (node.id.type !== 'ObjectPattern') return;
 	for (const p of node.id.properties) {
 		if (
 			p.type === 'Property' &&
 			p.value.type === 'Identifier' &&
-			!EXPECTED_PROP_NAMES.includes(p.value.name)
+			!expectedPropNames.includes(p.value.name)
 		) {
 			context.report({
 				node: p.value,
@@ -42,6 +49,8 @@ export default createRule('valid-prop-names-in-kit-pages', {
 	},
 	create(context) {
 		let isScript = false;
+		const isSvelte5 = getSvelteVersion(getFilename(context)) === '5';
+		const expectedPropNames = isSvelte5 ? EXPECTED_PROP_NAMES_SVELTE5 : EXPECTED_PROP_NAMES;
 		return {
 			// <script>
 			'Program > SvelteScriptElement > SvelteStartTag': (node: AST.SvelteStartTag) => {
@@ -67,7 +76,7 @@ export default createRule('valid-prop-names-in-kit-pages', {
 
 				// export let foo
 				if (node.id.type === 'Identifier') {
-					if (!EXPECTED_PROP_NAMES.includes(node.id.name)) {
+					if (!expectedPropNames.includes(node.id.name)) {
 						context.report({
 							node,
 							loc: node.loc,
@@ -78,7 +87,7 @@ export default createRule('valid-prop-names-in-kit-pages', {
 				}
 
 				// export let { xxx, yyy } = zzz
-				checkProp(node, context);
+				checkProp(node, context, expectedPropNames);
 			},
 
 			// Svelte5
@@ -93,7 +102,7 @@ export default createRule('valid-prop-names-in-kit-pages', {
 					return;
 				}
 
-				checkProp(node, context);
+				checkProp(node, context, expectedPropNames);
 			}
 		};
 	}

diff --git a/packages/eslint-plugin-svelte/src/utils/svelte-context.ts b/packages/eslint-plugin-svelte/src/utils/svelte-context.ts
@@ -3,6 +3,7 @@ import fs from 'fs';
 import path from 'path';
 import { getPackageJsons } from './get-package-json.js';
 import { getFilename, getSourceCode } from './compat.js';
+import { createCache } from './cache.js';
 
 const isRunInBrowser = !fs.readFileSync;
 
@@ -96,23 +97,51 @@ function getSvelteKitFileTypeFromFilePath(filePath: string): SvelteContext['svel
 	}
 }
 
+function extractMajorVersion(version: string, recognizePrereleaseVersion: boolean): string | null {
+	if (recognizePrereleaseVersion) {
+		const match = /^(?:\^|~)?(\d+\.0\.0-next)/.exec(version);
+		if (match && match[1]) {
+			return match[1];
+		}
+	}
+
+	const match = /^(?:\^|~)?(\d+)\./.exec(version);
+	if (match && match[1]) {
+		return match[1];
+	}
+	return null;
+}
+
+const svelteKitContextCache = createCache<Pick<
+	SvelteContext,
+	'svelteKitFileType' | 'svelteKitVersion'
+> | null>();
+
 function getSvelteKitContext(
 	context: RuleContext
 ): Pick<SvelteContext, 'svelteKitFileType' | 'svelteKitVersion'> {
 	const filePath = getFilename(context);
+
+	const cached = svelteKitContextCache.get(filePath);
+	if (cached) return cached;
+
 	const svelteKitVersion = getSvelteKitVersion(filePath);
 	if (svelteKitVersion == null) {
-		return {
+		const result: Pick<SvelteContext, 'svelteKitFileType' | 'svelteKitVersion'> = {
 			svelteKitFileType: null,
 			svelteKitVersion: null
 		};
+		svelteKitContextCache.set(filePath, result);
+		return result;
 	}
 	if (isRunInBrowser) {
-		return {
+		const result: Pick<SvelteContext, 'svelteKitFileType' | 'svelteKitVersion'> = {
 			svelteKitVersion,
 			// Judge by only file path if it runs in browser.
 			svelteKitFileType: getSvelteKitFileTypeFromFilePath(filePath)
 		};
+		svelteKitContextCache.set(filePath, result);
+		return result;
 	}
 
 	const routes =
@@ -123,21 +152,34 @@ function getSvelteKitContext(
 	const projectRootDir = getProjectRootDir(getFilename(context)) ?? '';
 
 	if (!filePath.startsWith(path.join(projectRootDir, routes))) {
-		return {
+		const result: Pick<SvelteContext, 'svelteKitFileType' | 'svelteKitVersion'> = {
 			svelteKitVersion,
 			svelteKitFileType: null
 		};
+		svelteKitContextCache.set(filePath, result);
+		return result;
 	}
 
-	return {
+	const result: Pick<SvelteContext, 'svelteKitFileType' | 'svelteKitVersion'> = {
 		svelteKitVersion,
 		svelteKitFileType: getSvelteKitFileTypeFromFilePath(filePath)
 	};
+	svelteKitContextCache.set(filePath, result);
+	return result;
 }
 
-function getSvelteVersion(filePath: string): SvelteContext['svelteVersion'] {
+const svelteVersionCache = createCache<SvelteContext['svelteVersion']>();
+
+export function getSvelteVersion(filePath: string): SvelteContext['svelteVersion'] {
+	const cached = svelteVersionCache.get(filePath);
+	if (cached) return cached;
+
 	// Hack: if it runs in browser, it regards as Svelte project.
-	if (isRunInBrowser) return '5';
+	if (isRunInBrowser) {
+		svelteVersionCache.set(filePath, '5');
+		return '5';
+	}
+
 	try {
 		const packageJsons = getPackageJsons(filePath);
 		for (const packageJson of packageJsons) {
@@ -147,17 +189,22 @@ function getSvelteVersion(filePath: string): SvelteContext['svelteVersion'] {
 			}
 			const major = extractMajorVersion(version, false);
 			if (major === '3' || major === '4') {
+				svelteVersionCache.set(filePath, '3/4');
 				return '3/4';
 			}
+			svelteVersionCache.set(filePath, major as SvelteContext['svelteVersion']);
 			return major as SvelteContext['svelteVersion'];
 		}
 	} catch {
 		/** do nothing */
 	}
 
+	svelteVersionCache.set(filePath, null);
 	return null;
 }
 
+const svelteKitVersionCache = createCache<SvelteContext['svelteKitVersion']>();
+
 /**
  * Check givin file is under SvelteKit project.
  *
@@ -167,14 +214,22 @@ function getSvelteVersion(filePath: string): SvelteContext['svelteVersion'] {
  * @returns
  */
 function getSvelteKitVersion(filePath: string): SvelteContext['svelteKitVersion'] {
+	const cached = svelteKitVersionCache.get(filePath);
+	if (cached) return cached;
+
 	// Hack: if it runs in browser, it regards as SvelteKit project.
-	if (isRunInBrowser) return '2';
+	if (isRunInBrowser) {
+		svelteKitVersionCache.set(filePath, '2');
+		return '2';
+	}
+
 	try {
 		const packageJsons = getPackageJsons(filePath);
 		if (packageJsons.length === 0) return null;
 		if (packageJsons[0].name === 'eslint-plugin-svelte') {
 			// Hack: CI removes `@sveltejs/kit` and it returns false and test failed.
 			// So always it returns 2 if it runs on the package.
+			svelteKitVersionCache.set(filePath, '2');
 			return '2';
 		}
 
@@ -183,32 +238,22 @@ function getSvelteKitVersion(filePath: string): SvelteContext['svelteKitVersion'
 				packageJson.dependencies?.['@sveltejs/kit'] ??
 				packageJson.devDependencies?.['@sveltejs/kit'];
 			if (typeof version !== 'string') {
+				svelteKitVersionCache.set(filePath, null);
 				return null;
 			}
-
-			return extractMajorVersion(version, true) as SvelteContext['svelteKitVersion'];
+			const major = extractMajorVersion(version, true) as SvelteContext['svelteKitVersion'];
+			svelteKitVersionCache.set(filePath, major);
+			return major;
 		}
 	} catch {
 		/** do nothing */
 	}
 
+	svelteKitVersionCache.set(filePath, null);
 	return null;
 }
 
-function extractMajorVersion(version: string, recognizePrereleaseVersion: boolean): string | null {
-	if (recognizePrereleaseVersion) {
-		const match = /^(?:\^|~)?(\d+\.0\.0-next)/.exec(version);
-		if (match && match[1]) {
-			return match[1];
-		}
-	}
-
-	const match = /^(?:\^|~)?(\d+)\./.exec(version);
-	if (match && match[1]) {
-		return match[1];
-	}
-	return null;
-}
+const projectRootDirCache = createCache<string | null>();
 
 /**
  * Gets a  project root folder path.
@@ -217,58 +262,81 @@ function extractMajorVersion(version: string, recognizePrereleaseVersion: boolea
  */
 function getProjectRootDir(filePath: string): string | null {
 	if (isRunInBrowser) return null;
+	const cached = projectRootDirCache.get(filePath);
+	if (cached) return cached;
+
 	const packageJsons = getPackageJsons(filePath);
 	if (packageJsons.length === 0) {
+		projectRootDirCache.set(filePath, null);
 		return null;
 	}
 	const packageJsonFilePath = packageJsons[0].filePath;
-	if (!packageJsonFilePath) return null;
-	return path.dirname(path.resolve(packageJsonFilePath));
+	if (!packageJsonFilePath) {
+		projectRootDirCache.set(filePath, null);
+		return null;
+	}
+	const projectRootDir = path.dirname(path.resolve(packageJsonFilePath));
+	projectRootDirCache.set(filePath, projectRootDir);
+	return projectRootDir;
 }
 
+const svelteContextCache = createCache<SvelteContext | null>();
+
 export function getSvelteContext(context: RuleContext): SvelteContext | null {
 	const { parserServices } = getSourceCode(context);
 	const { svelteParseContext } = parserServices;
 	const filePath = getFilename(context);
+
+	const cached = svelteContextCache.get(filePath);
+	if (cached) return cached;
+
 	const svelteKitContext = getSvelteKitContext(context);
 	const svelteVersion = getSvelteVersion(filePath);
 	const svelteFileType = getSvelteFileType(filePath);
 
 	if (svelteVersion == null) {
-		return {
+		const result: SvelteContext = {
 			svelteVersion: null,
 			svelteFileType: null,
 			runes: null,
 			svelteKitVersion: svelteKitContext.svelteKitVersion,
 			svelteKitFileType: svelteKitContext.svelteKitFileType
 		};
+		svelteContextCache.set(filePath, result);
+		return result;
 	}
 
 	if (svelteVersion === '3/4') {
-		return {
+		const result: SvelteContext = {
 			svelteVersion,
 			svelteFileType: svelteFileType === '.svelte' ? '.svelte' : null,
 			runes: null,
 			svelteKitVersion: svelteKitContext.svelteKitVersion,
 			svelteKitFileType: svelteKitContext.svelteKitFileType
 		};
+		svelteContextCache.set(filePath, result);
+		return result;
 	}
 
 	if (svelteFileType == null) {
-		return {
+		const result: SvelteContext = {
 			svelteVersion,
 			svelteFileType: null,
 			runes: null,
 			svelteKitVersion: svelteKitContext.svelteKitVersion,
 			svelteKitFileType: svelteKitContext.svelteKitFileType
 		};
+		svelteContextCache.set(filePath, result);
+		return result;
 	}
 
-	return {
+	const result: SvelteContext = {
 		svelteVersion,
 		runes: svelteParseContext?.runes ?? 'undetermined',
 		svelteFileType,
 		svelteKitVersion: svelteKitContext.svelteKitVersion,
 		svelteKitFileType: svelteKitContext.svelteKitFileType
 	};
+	svelteContextCache.set(filePath, result);
+	return result;
 }
diff --git a/.../tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/+page.svelte b/.../tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/+page.svelte
@@ -0,0 +1,5 @@
+<script>
+	export let children;
+</script>
+
+{children}
diff --git a/.../tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/_config.json b/.../tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/_config.json
@@ -0,0 +1,13 @@
+{
+	"languageOptions": {
+		"parserOptions": {
+			"svelteConfig": {
+				"kit": {
+					"files": {
+						"routes": "tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children"
+					}
+				}
+			}
+		}
+	}
+}
diff --git a/...e/tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/errors.yaml b/...e/tests/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/errors.yaml
@@ -0,0 +1,4 @@
+- message: disallow props other than data or errors in SvelteKit page components.
+  line: 2
+  column: 13
+  suggestions: null
diff --git a/...s/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/requirements.json b/...s/fixtures/rules/valid-prop-names-in-kit-pages/invalid/svelte4-children/requirements.json
@@ -0,0 +1,3 @@
+{
+	"svelte": "^3.0.0 || ^4.0.0"
+}
diff --git a/...sts/fixtures/rules/valid-prop-names-in-kit-pages/valid/svelte5-without-runes/+page.svelte b/...sts/fixtures/rules/valid-prop-names-in-kit-pages/valid/svelte5-without-runes/+page.svelte
@@ -0,0 +1,24 @@
+<script>
+	export let data;
+	export let errors;
+	export let form;
+	export let children;
+
+	let comment = '';
+
+	export const snapshot = {
+		capture: () => comment,
+		restore: (value) => (comment = value)
+	};
+</script>
+
+{data}, {errors}
+
+{#if form?.success}
+	<p>Successfully logged in! Welcome back, {data.user.name}</p>
+{/if}
+
+<form method="POST">
+	<textarea bind:value={comment} />
+	<button>Post comment</button>
+</form>