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: migrate svelte:self #13504

Merged
merged 13 commits into from
Oct 5, 2024
65 changes: 57 additions & 8 deletions packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ const style_placeholder = '/*$$__STYLE_CONTENT__$$*/';
* May throw an error if the code is too complex to migrate automatically.
*
* @param {string} source
* @param {{filename?: string}} [options]
* @returns {{ code: string; }}
*/
export function migrate(source) {
export function migrate(source, { filename } = {}) {
try {
// Blank CSS, could contain SCSS or similar that needs a preprocessor.
// Since we don't care about CSS in this migration, we'll just ignore it.
Expand All @@ -41,7 +42,7 @@ export function migrate(source) {
});

reset_warning_filter(() => false);
reset(source, { filename: 'migrate.svelte' });
reset(source, { filename: filename ?? 'migrate.svelte' });

let parsed = parse(source);

Expand All @@ -68,6 +69,7 @@ export function migrate(source) {
let state = {
scope: analysis.instance.scope,
analysis,
filename,
str,
indent,
props: [],
Expand All @@ -90,12 +92,14 @@ export function migrate(source) {
createBubbler: analysis.root.unique('createBubbler').name,
bubble: analysis.root.unique('bubble').name,
passive: analysis.root.unique('passive').name,
nonpassive: analysis.root.unique('nonpassive').name
nonpassive: analysis.root.unique('nonpassive').name,
svelte_self: analysis.root.unique('SvelteSelf').name
},
legacy_imports: new Set(),
script_insertions: new Set(),
derived_components: new Map(),
derived_labeled_statements: new Set()
derived_labeled_statements: new Set(),
has_svelte_self: false
};

if (parsed.module) {
Expand All @@ -122,12 +126,21 @@ export function migrate(source) {
state.script_insertions.size > 0 ||
state.props.length > 0 ||
analysis.uses_rest_props ||
analysis.uses_props;
analysis.uses_props ||
state.has_svelte_self;

if (!parsed.instance && need_script) {
str.appendRight(0, '<script>');
}

if (state.has_svelte_self && filename) {
const file = filename.split('/').pop();
str.appendRight(
insertion_point,
`\n${indent}import ${state.names.svelte_self} from './${file}';`
);
}

const specifiers = [...state.legacy_imports].map((imported) => {
const local = state.names[imported];
return imported === local ? imported : `${imported} as ${local}`;
Expand Down Expand Up @@ -298,6 +311,7 @@ export function migrate(source) {
* scope: Scope;
* str: MagicString;
* analysis: ComponentAnalysis;
* filename?: string;
* indent: string;
* props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string; type_only?: boolean; needs_refine_type?: boolean; }>;
* props_insertion_point: number;
Expand All @@ -306,8 +320,9 @@ export function migrate(source) {
* names: Record<string, string>;
* legacy_imports: Set<string>;
* script_insertions: Set<string>;
* derived_components: Map<string, string>,
* derived_labeled_statements: Set<LabeledStatement>
* derived_components: Map<string, string>;
* derived_labeled_statements: Set<LabeledStatement>;
* has_svelte_self: boolean;
* }} State
*/

Expand Down Expand Up @@ -729,9 +744,43 @@ const template = {
}
next();
},
SvelteSelf(node, { state, next }) {
const source = state.str.original.substring(node.start, node.end);
if (!state.filename) {
const indent = guess_indent(source);
state.str.prependRight(
node.start,
`<!-- @migration-task: svelte:self is deprecated, import this Svelte file into itself instead -->\n${indent}`
);
next();
return;
}
// overwrite the open tag
state.str.overwrite(
node.start + 1,
node.start + 1 + 'svelte:self'.length,
`${state.names.svelte_self}`
);
// if it has a fragment we need to overwrite the closing tag too
if (node.fragment.nodes.length > 0) {
const start_closing =
state.str.original.indexOf('<', node.fragment.nodes[node.fragment.nodes.length - 1].end) +
paoloricciuti marked this conversation as resolved.
Show resolved Hide resolved
2;
state.str.overwrite(start_closing, node.end - 1, `${state.names.svelte_self}`);
} else if (!source.endsWith('/>')) {
// special case for case `<svelte:self></svelte:self>` it has no fragment but
// we still need to overwrite the end tag
state.str.overwrite(
node.start + source.lastIndexOf('</', node.end) + 2,
node.end - 1,
`${state.names.svelte_self}`
);
}
state.has_svelte_self = true;
next();
},
SvelteElement(node, { state, path, next }) {
migrate_slot_usage(node, path, state);

if (node.tag.type === 'Literal') {
let is_static = true;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<script>
let SvelteSelf;
</script>

{#if false}
<svelte:self />
<svelte:self with_attributes/>
<svelte:self count={count+1}/>
<svelte:self>
child
</svelte:self>
<svelte:self count={count+1}>
child
</svelte:self>
<svelte:self count={$$props.count} >
child
</svelte:self>
<svelte:self></svelte:self>
{/if}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<script>
import SvelteSelf_1 from './output.svelte';
/** @type {Record<string, any>} */
let { ...props } = $props();
let SvelteSelf;
</script>

{#if false}
<SvelteSelf_1 />
<SvelteSelf_1 with_attributes/>
<SvelteSelf_1 count={count+1}/>
<SvelteSelf_1>
child
</SvelteSelf_1>
<SvelteSelf_1 count={count+1}>
child
</SvelteSelf_1>
<SvelteSelf_1 count={props.count} >
child
</SvelteSelf_1>
<SvelteSelf_1></SvelteSelf_1>
{/if}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
skip_filename: true
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{#if false}
<svelte:self />
{/if}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{#if false}
<!-- @migration-task: svelte:self is deprecated, import this Svelte file into itself instead -->
<svelte:self />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to just have this as a single test, no need to check all variants for a successful addition of a comment

{/if}
15 changes: 15 additions & 0 deletions packages/svelte/tests/migrate/samples/svelte-self/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{#if false}
<svelte:self />
<svelte:self with_attributes/>
<svelte:self count={count+1}/>
<svelte:self>
child
</svelte:self>
<svelte:self count={count+1}>
child
</svelte:self>
<svelte:self count={$$props.count} >
child
</svelte:self>
<svelte:self></svelte:self>
{/if}
21 changes: 21 additions & 0 deletions packages/svelte/tests/migrate/samples/svelte-self/output.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<script>
import SvelteSelf from './output.svelte';
/** @type {Record<string, any>} */
let { ...props } = $props();
</script>

{#if false}
<SvelteSelf />
<SvelteSelf with_attributes/>
<SvelteSelf count={count+1}/>
<SvelteSelf>
child
</SvelteSelf>
<SvelteSelf count={count+1}>
child
</SvelteSelf>
<SvelteSelf count={props.count} >
child
</SvelteSelf>
<SvelteSelf></SvelteSelf>
{/if}
8 changes: 6 additions & 2 deletions packages/svelte/tests/migrate/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@ import { migrate } from 'svelte/compiler';
import { try_read_file } from '../helpers.js';
import { suite, type BaseTest } from '../suite.js';

interface ParserTest extends BaseTest {}
interface ParserTest extends BaseTest {
skip_filename?: boolean;
}

const { test, run } = suite<ParserTest>(async (config, cwd) => {
const input = fs
.readFileSync(`${cwd}/input.svelte`, 'utf-8')
.replace(/\s+$/, '')
.replace(/\r/g, '');

const actual = migrate(input).code;
const actual = migrate(input, {
filename: config.skip_filename ? undefined : `${cwd}/output.svelte`
}).code;

// run `UPDATE_SNAPSHOTS=true pnpm test migrate` to update parser tests
if (process.env.UPDATE_SNAPSHOTS || !fs.existsSync(`${cwd}/output.svelte`)) {
Expand Down
4 changes: 3 additions & 1 deletion packages/svelte/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,9 @@ declare module 'svelte/compiler' {
* May throw an error if the code is too complex to migrate automatically.
*
* */
export function migrate(source: string): {
export function migrate(source: string, { filename }?: {
filename?: string;
} | undefined): {
code: string;
};
namespace Css {
Expand Down
3 changes: 2 additions & 1 deletion sites/svelte-5-preview/src/lib/Output/Compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export default class Compiler {
this.worker.postMessage({
id,
type: 'migrate',
source: file.source
source: file.source,
filename: `${file.name}.${file.type}`
});
});
}
Expand Down
4 changes: 2 additions & 2 deletions sites/svelte-5-preview/src/lib/workers/compiler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ function compile({ id, source, options, return_ast }) {
}

/** @param {import("../workers").MigrateMessageData} param0 */
function migrate({ id, source }) {
function migrate({ id, source, filename }) {
try {
const result = svelte.migrate(source);
const result = svelte.migrate(source, { filename });

return {
id,
Expand Down