[feat] Suggest props destructuring if possible (#6069)

Should make migration easier in most cases

.changeset/ninety-snakes-melt.md

@@ -0,0 +1,5 @@
+---
+'svelte-migrate': patch
+---
+
+Suggest props destructuring if possible

packages/migrate/migrations/routes/index.js

@@ -148,7 +148,12 @@ export async function migrate() {
 
 			renamed += svelte_ext;
 
-			const { module, main, ext } = migrate_scripts(content, is_error_page, move_to_directory);
+			const { module, main, ext } = migrate_scripts(
+				content,
+				bare,
+				is_error_page,
+				move_to_directory
+			);
 
 			if (move_to_directory) {
 				const dir = path.dirname(renamed);

packages/migrate/migrations/routes/migrate_scripts/index.js

@@ -9,16 +9,21 @@ import {
 	except_str
 } from '../utils.js';
 import * as TASKS from '../tasks.js';
+import MagicString from 'magic-string';
 
 /**
  * @param {string} content
+ * @param {string} filename
  * @param {boolean} is_error
  * @param {boolean} moved
  */
-export function migrate_scripts(content, is_error, moved) {
+export function migrate_scripts(content, filename, is_error, moved) {
 	/** @type {string | null} */
 	let module = null;
 
+	const match = /__layout(?:-([^.@]+))?/.exec(filename);
+	const data_name = match?.[1] ? `LayoutData.${match[1]}` : match ? `LayoutData` : 'PageData';
+
 	let ext = '.js';
 
 	// instance script
@@ -31,6 +36,10 @@ export function migrate_scripts(content, is_error, moved) {
 				contents = adjust_imports(contents);
 			}
 
+			if (/lang(?:uage)?=(['"])(ts|typescript)\1/.test(attrs)) {
+				ext = '.ts';
+			}
+
 			if (/context=(['"])module\1/.test(attrs)) {
 				// special case — load is no longer supported in error
 				if (is_error) {
@@ -42,10 +51,6 @@ export function migrate_scripts(content, is_error, moved) {
 					return `<script${attrs}>${body}</script>${whitespace}`;
 				}
 
-				if (/lang(?:uage)?=(['"])(ts|typescript)\1/.test(attrs)) {
-					ext = '.ts';
-				}
-
 				module = dedent(contents.replace(/^\n/, ''));
 
 				const declared = find_declarations(contents);
@@ -88,8 +93,22 @@ export function migrate_scripts(content, is_error, moved) {
 			}
 
 			if (!is_error && /export/.test(contents)) {
-				contents = `\n${indent}${error('Add data prop', TASKS.PAGE_DATA_PROP)}\n${contents}`;
-				// Possible TODO: migrate props to data.prop, or suggest $: ({propX, propY, ...} = data);
+				let prefix = `\n${indent}${error('Add data prop', TASKS.PAGE_DATA_PROP)}`;
+				const props = get_props(contents);
+				if (props) {
+					prefix += `\n${comment(
+						`${indent}Suggestion (check code before using, and possibly convert to data.X access later):\n` +
+							`${indent}${
+								ext === '.ts'
+									? `import type { ${data_name} } from './$types';`
+									: `/** @type {import('./$types').${data_name}} */`
+							}\n` +
+							`${indent}export let data${ext === '.ts' ? `: ${data_name}` : ''};\n` +
+							`${indent}$: ({ ${props.join(', ')} } = data);`,
+						indent
+					)}`;
+				}
+				contents = `${prefix}\n${contents}`;
 			}
 
 			return `<script${attrs}>${contents}</script>${whitespace}`;
@@ -155,3 +174,74 @@ function find_declarations(content) {
 
 	return declared;
 }
+
+/**
+ * @param {string} content
+ */
+function get_props(content) {
+	try {
+		const ast = ts.createSourceFile(
+			'filename.ts',
+			content,
+			ts.ScriptTarget.Latest,
+			true,
+			ts.ScriptKind.TS
+		);
+
+		const code = new MagicString(content);
+		let give_up = false;
+		/** @type {string[] } */
+		let exports = [];
+
+		/** @param {ts.Node} node */
+		function walk(node) {
+			if (
+				ts.isExportDeclaration(node) ||
+				((ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node)) && hasExportKeyword(node))
+			) {
+				give_up = true;
+				return;
+			}
+
+			if (ts.isVariableStatement(node)) {
+				if (hasExportKeyword(node)) {
+					const isLet = node.declarationList.flags === ts.NodeFlags.Let;
+					if (isLet) {
+						ts.forEachChild(node.declarationList, (node) => {
+							if (ts.isVariableDeclaration(node)) {
+								if (ts.isIdentifier(node.name)) {
+									const name = node.name.getText();
+									if (name === 'data') {
+										give_up = true;
+										return;
+									} else {
+										exports.push(name);
+									}
+								} else {
+									give_up = true;
+									return;
+								}
+							}
+						});
+					} else {
+						give_up = true;
+						return;
+					}
+				}
+			}
+		}
+
+		ts.forEachChild(ast, walk);
+		if (give_up) return;
+		return exports;
+	} catch (e) {
+		return;
+	}
+}
+
+/**
+ * @param {ts.Node} node
+ */
+export function hasExportKeyword(node) {
+	return node.modifiers?.find((x) => x.kind == ts.SyntaxKind.ExportKeyword);
+}

packages/migrate/migrations/routes/migrate_scripts/index.spec.js

@@ -7,6 +7,7 @@ for (const sample of read_samples(import.meta.url)) {
 	test(sample.description, () => {
 		const actual = migrate_scripts(
 			sample.before,
+			sample.filename ?? 'some-page.svelte',
 			sample.description.includes('error'),
 			sample.description.includes('moved')
 		);

packages/migrate/migrations/routes/migrate_scripts/samples.md

@@ -77,13 +77,19 @@
 ```svelte after
 <script>
 	throw new Error("@migration task: Add data prop (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292707)");
+	// Suggestion (check code before using, and possibly convert to data.X access later):
+	// /** @type {import('./$types').PageData} */
+	// export let data;
+	// $: ({ a } = data);
 
 	export let a;
 </script>
 ```
 
 ## Module context with moved imports
 
+> file: __layout.svelte
+
 ```svelte before
 <script context="module">
 	import Foo from './Foo.svelte';
@@ -96,7 +102,7 @@
 	}
 </script>
 
-<script>
+<script lang="ts">
 	export let sry;
 </script>
 
@@ -117,8 +123,12 @@
 	// }
 </script>
 
-<script>
+<script lang="ts">
 	throw new Error("@migration task: Add data prop (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292707)");
+	// Suggestion (check code before using, and possibly convert to data.X access later):
+	// import type { LayoutData } from './$types';
+	// export let data: LayoutData;
+	// $: ({ sry } = data);
 
 	export let sry;
 </script>