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

feat: don't print errors on migration errors #13754

Merged
merged 2 commits into from
Oct 21, 2024
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/large-carrots-behave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't print errors on migration errors
25 changes: 18 additions & 7 deletions packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ const style_placeholder = '/*$$__STYLE_CONTENT__$$*/';

let has_migration_task = false;

class MigrationError extends Error {
/**
* @param {string} msg
*/
constructor(msg) {
super(msg);
}
}

/**
* Does a best-effort migration of Svelte code towards using runes, event attributes and render tags.
* May throw an error if the code is too complex to migrate automatically.
Expand Down Expand Up @@ -310,8 +319,10 @@ export function migrate(source, { filename } = {}) {
}
return { code: str.toString() };
} catch (e) {
// eslint-disable-next-line no-console
console.error('Error while migrating Svelte code', e);
if (!(e instanceof MigrationError)) {
// eslint-disable-next-line no-console
console.error('Error while migrating Svelte code', e);
}
has_migration_task = true;
return {
code: `<!-- @migration-task Error while migrating Svelte code: ${/** @type {any} */ (e).message} -->\n${og_source}`
Expand Down Expand Up @@ -398,7 +409,7 @@ const instance_script = {
state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end));
}
if (illegal_specifiers.length > 0) {
throw new Error(
throw new MigrationError(
`Can't migrate code with ${illegal_specifiers.join(' and ')}. Please migrate by hand.`
);
}
Expand Down Expand Up @@ -462,7 +473,7 @@ const instance_script = {

if (declarator.id.type !== 'Identifier') {
// TODO invest time in this?
throw new Error(
throw new MigrationError(
'Encountered an export declaration pattern that is not supported for automigration.'
);
// Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = ..
Expand Down Expand Up @@ -493,7 +504,7 @@ const instance_script = {
const binding = /** @type {Binding} */ (state.scope.get(name));

if (state.analysis.uses_props && (declarator.init || binding.updated)) {
throw new Error(
throw new MigrationError(
'$$props is used together with named props in a way that cannot be automatically migrated.'
);
}
Expand Down Expand Up @@ -1065,7 +1076,7 @@ const template = {
} else if (slot_name !== 'default') {
name = state.scope.generate(slot_name);
if (name !== slot_name) {
throw new Error(
throw new MigrationError(
'This migration would change the name of a slot making the component unusable'
);
}
Expand Down Expand Up @@ -1520,7 +1531,7 @@ function handle_identifier(node, state, path) {
} else if (name !== 'default') {
let new_name = state.scope.generate(name);
if (new_name !== name) {
throw new Error(
throw new MigrationError(
'This migration would change the name of a slot making the component unusable'
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import { test } from '../../test';
export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
]
],
errors: []
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import { test } from '../../test';
export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
]
],
errors: []
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import { test } from '../../test';
export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
]
],
errors: []
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import { test } from '../../test';
export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
]
],
errors: []
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import { test } from '../../test';
export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
]
],
errors: []
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { test } from '../../test';

export default test({
logs: [
'One or more `@migration-task` comments were added to `output.svelte`, please check them and complete the migration manually.'
],
errors: [
'Error while migrating Svelte code',
{
code: 'unexpected_eof',
end: {
character: 30,
column: 21,
line: 3
},
filename: 'output.svelte',
frame: `1: <script
2:
3: unterminated template
^`,
message: 'Unexpected end of input',
name: 'CompileError',
position: [30, 30],
start: {
character: 30,
column: 21,
line: 3
}
}
]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script

unterminated template
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<!-- @migration-task Error while migrating Svelte code: Unexpected end of input -->
<script

unterminated template
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ export default test({
skip_filename: true,
logs: [
"One or more `@migration-task` comments were added to a file (unfortunately we don't know the name), please check them and complete the migration manually."
]
],
errors: []
});
14 changes: 13 additions & 1 deletion packages/svelte/tests/migrate/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { suite, type BaseTest } from '../suite.js';

interface ParserTest extends BaseTest {
skip_filename?: boolean;
logs?: string[];
logs?: any[];
errors?: any[];
}

const { test, run } = suite<ParserTest>(async (config, cwd) => {
Expand All @@ -16,13 +17,20 @@ const { test, run } = suite<ParserTest>(async (config, cwd) => {
.replace(/\r/g, '');

const logs: any[] = [];
const errors: any[] = [];

if (config.logs) {
console.log = (...args) => {
logs.push(...args);
};
}

if (config.errors) {
console.error = (...args) => {
errors.push(...args);
};
}

const actual = migrate(input, {
filename: config.skip_filename ? undefined : `output.svelte`
}).code;
Expand All @@ -31,6 +39,10 @@ const { test, run } = suite<ParserTest>(async (config, cwd) => {
assert.deepEqual(logs, config.logs);
}

if (config.errors) {
assert.deepEqual(errors, config.errors);
}

// run `UPDATE_SNAPSHOTS=true pnpm test migrate` to update parser tests
if (process.env.UPDATE_SNAPSHOTS || !fs.existsSync(`${cwd}/output.svelte`)) {
fs.writeFileSync(`${cwd}/output.svelte`, actual);
Expand Down