Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] check load function input #5838

Merged
merged 1 commit into from
Aug 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wise-hotels-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte-migrate': patch
---

check load function input
58 changes: 58 additions & 0 deletions packages/migrate/migrations/routes/migrate_page_js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
get_object_nodes,
is_new,
is_string_like,
manual_migration,
manual_return_migration,
parse,
rewrite_returns
Expand Down Expand Up @@ -34,6 +35,8 @@ export function migrate_page(content) {
for (const statement of file.ast.statements) {
const fn = get_function_node(statement, name);
if (fn) {
check_fn_param(fn, file.code);

/** @type {Set<string>} */
const imports = new Set();

Expand All @@ -43,6 +46,10 @@ export function migrate_page(content) {
if (nodes) {
const keys = Object.keys(nodes).sort().join(' ');

if (keys === '') {
return; // nothing to do
}

if (keys === 'props') {
automigration(expr, file.code, dedent(nodes.props.getText()));
return;
Expand Down Expand Up @@ -102,3 +109,54 @@ export function migrate_page(content) {
// an error at the top of the file
return give_up + content;
}

/**
* Check the load function parameter, and either adjust
* the property names, or add an error
* @param {ts.FunctionDeclaration | ts.FunctionExpression | ts.ArrowFunction} fn
* @param {import('magic-string').default} str
*/
function check_fn_param(fn, str) {
const param = fn.parameters[0];
if (!param) {
return;
}
if (ts.isObjectBindingPattern(param.name)) {
for (const binding of param.name.elements) {
if (
!ts.isIdentifier(binding.name) ||
(binding.propertyName && !ts.isIdentifier(binding.propertyName))
) {
bail(true);
return;
}
const name = binding.propertyName
? /** @type {ts.Identifier} */ (binding.propertyName).text
: binding.name.text;
if (['stuff', 'status', 'error'].includes(name)) {
bail();
return;
}
if (name === 'props') {
if (binding.propertyName) {
bail();
return;
} else {
str.overwrite(binding.name.getStart(), binding.name.getEnd(), 'data: props');
}
}
}
} else {
// load(param) { .. } -> bail, we won't check what is used in the function body
bail(true);
}

function bail(check = false) {
manual_migration(
fn,
str,
(check ? 'Check if you need to migrate' : 'Migrate') + ' the load function input',
TASKS.PAGE_LOAD
);
}
}
46 changes: 45 additions & 1 deletion packages/migrate/migrations/routes/migrate_page_js/samples.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,48 @@ export async function load({ fetch }) {
throw new Error("@migration task: Migrate this return statement (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292693)");
return await res.json();
}
```
```

## Renames props -> data, leaves unchanged alone

```js before
export async function load({ props, params }) {
return {};
}
```

```js after
export async function load({ data: props, params }) {
return {};
}
```

## Errors on stuff

```js before
export async function load({ stuff }) {
return {};
}
```

```js after
throw new Error("@migration task: Migrate the load function input (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292693)");
export async function load({ stuff }) {
return {};
}
```

## Bails on non-destructured param

```js before
export async function load(input) {
return {};
}
```

```js after
throw new Error("@migration task: Check if you need to migrate the load function input (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292693)");
export async function load(input) {
return {};
}
```
16 changes: 12 additions & 4 deletions packages/migrate/migrations/routes/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,17 @@ export function manual_return_migration(node, str, comment_nr, suggestion) {
node = node.parent.parent.parent;
}

manual_migration(node, str, 'Migrate this return statement', comment_nr, suggestion);
}

/**
* @param {ts.Node} node
* @param {MagicString} str
* @param {string} message
* @param {string} comment_nr
* @param {string} [suggestion]
*/
export function manual_migration(node, str, message, comment_nr, suggestion) {
const indent = indent_at_line(str.original, node.getStart());

let appended = '';
Expand All @@ -219,10 +230,7 @@ export function manual_return_migration(node, str, comment_nr, suggestion) {
)}`;
}

str.prependLeft(
node.getStart(),
error('Migrate this return statement', comment_nr) + appended + `\n${indent}`
);
str.prependLeft(node.getStart(), error(message, comment_nr) + appended + `\n${indent}`);
}

/**
Expand Down