Skip to content

Commit

Permalink
Revert "[feat] Suggest props destructuring if possible (#6069)"
Browse files Browse the repository at this point in the history
This reverts commit b1dafb8.
  • Loading branch information
Rich-Harris authored Aug 19, 2022
1 parent 0467aea commit dda9004
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 121 deletions.
5 changes: 0 additions & 5 deletions .changeset/ninety-snakes-melt.md

This file was deleted.

7 changes: 1 addition & 6 deletions packages/migrate/migrations/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,7 @@ export async function migrate() {

renamed += svelte_ext;

const { module, main, ext } = migrate_scripts(
content,
bare,
is_error_page,
move_to_directory
);
const { module, main, ext } = migrate_scripts(content, is_error_page, move_to_directory);

if (move_to_directory) {
const dir = path.dirname(renamed);
Expand Down
104 changes: 7 additions & 97 deletions packages/migrate/migrations/routes/migrate_scripts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,16 @@ 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, filename, is_error, moved) {
export function migrate_scripts(content, 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
Expand All @@ -36,10 +31,6 @@ export function migrate_scripts(content, filename, 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) {
Expand All @@ -51,6 +42,10 @@ export function migrate_scripts(content, filename, 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);
Expand Down Expand Up @@ -93,22 +88,8 @@ export function migrate_scripts(content, filename, is_error, moved) {
}

if (!is_error && /export/.test(contents)) {
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}`;
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);
}

return `<script${attrs}>${contents}</script>${whitespace}`;
Expand Down Expand Up @@ -174,74 +155,3 @@ 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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')
);
Expand Down
14 changes: 2 additions & 12 deletions packages/migrate/migrations/routes/migrate_scripts/samples.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,13 @@
```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';
Expand All @@ -102,7 +96,7 @@
}
</script>
<script lang="ts">
<script>
export let sry;
</script>
Expand All @@ -123,12 +117,8 @@
// }
</script>
<script lang="ts">
<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):
// import type { LayoutData } from './$types';
// export let data: LayoutData;
// $: ({ sry } = data);
export let sry;
</script>
Expand Down

0 comments on commit dda9004

Please sign in to comment.