From 789cd210e8b1e9d9e9663eff0ee7d772c3a85685 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 17 Jan 2025 10:25:21 +0100 Subject: [PATCH] fix: take other snippets into account when checking for hoistability --- .../svelte2tsx/src/htmlxtojsx_v2/index.ts | 11 +++++-- packages/svelte2tsx/src/svelte2tsx/index.ts | 4 +++ .../svelte2tsx/nodes/HoistableInterfaces.ts | 22 ++++++++++++++ .../snippet-module-hoist-4.v5/expectedv2.ts | 30 +++++++++++++++++++ .../snippet-module-hoist-4.v5/input.svelte | 23 ++++++++++++++ 5 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/input.svelte diff --git a/packages/svelte2tsx/src/htmlxtojsx_v2/index.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/index.ts index 2e5ac14d3..65a06be0a 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/index.ts @@ -54,7 +54,7 @@ export interface TemplateProcessResult { scriptTag: BaseNode; moduleScriptTag: BaseNode; /** Start/end positions of snippets that should be moved to the instance script or possibly even module script */ - rootSnippets: Array<[start: number, end: number, globals: Map]>; + rootSnippets: Array<[start: number, end: number, globals: Map, string]>; /** To be added later as a comment on the default class export */ componentDocumentation: ComponentDocumentation; events: ComponentEvents; @@ -93,7 +93,7 @@ export function convertHtmlxToJsx( stripDoctype(str); - const rootSnippets: Array<[number, number, Map]> = []; + const rootSnippets: Array<[number, number, Map, string]> = []; let element: Element | InlineComponent | undefined; const pendingSnippetHoistCheck = new Set(); @@ -264,7 +264,12 @@ export function convertHtmlxToJsx( } }); - rootSnippets.push([node.start, node.end, result.globals]); + rootSnippets.push([ + node.start, + node.end, + result.globals, + node.expression.name + ]); } else { pendingSnippetHoistCheck.add(parent); } diff --git a/packages/svelte2tsx/src/svelte2tsx/index.ts b/packages/svelte2tsx/src/svelte2tsx/index.ts index 207903fa0..a3b45221f 100644 --- a/packages/svelte2tsx/src/svelte2tsx/index.ts +++ b/packages/svelte2tsx/src/svelte2tsx/index.ts @@ -165,6 +165,10 @@ export function svelte2tsx( } } + if (moduleScriptTag && rootSnippets.length > 0) { + exportedNames.hoistableInterfaces.analyzeSnippets(rootSnippets); + } + if (moduleScriptTag || scriptTag) { for (const [start, end, globals] of rootSnippets) { const hoist_to_module = diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts index 8fd726ff6..7d95d1c73 100644 --- a/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts +++ b/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts @@ -19,6 +19,28 @@ export class HoistableInterfaces { value_deps: new Set() }; + analyzeSnippets( + rootSnippets: [start: number, end: number, globals: Map, string][] + ) { + let prev_disallowed_values_size; + // we need to recalculate the disallowed values until they are stable because + // one snippet might depend on another snippet which was previously hoistable + while ( + prev_disallowed_values_size == null || + this.disallowed_values.size !== prev_disallowed_values_size + ) { + prev_disallowed_values_size = this.disallowed_values.size; + for (const [, , globals, name] of rootSnippets) { + const hoist_to_module = + globals.size === 0 || + [...globals.keys()].every((id) => this.isAllowedReference(id)); + if (!hoist_to_module) { + this.disallowed_values.add(name); + } + } + } + } + /** should be called before analyzeInstanceScriptNode */ analyzeModuleScriptNode(node: ts.Node) { // Handle Import Declarations diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/expectedv2.ts new file mode 100644 index 000000000..934387755 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/expectedv2.ts @@ -0,0 +1,30 @@ +/// +; + +;; const hoistable/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => { + { svelteHTML.createElement("h1", {}); } +};return __sveltets_2_any(0)};function render() { + const chain/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => { + { svelteHTML.createElement("div", {});foo; } +};return __sveltets_2_any(0)}; const chain2/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => { + ;__sveltets_2_ensureSnippet(chain()); +};return __sveltets_2_any(0)}; const chain3/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => { + ;__sveltets_2_ensureSnippet(chain2()); +};return __sveltets_2_any(0)}; + let foo = true; +; +async () => { + + + + + + + + + +}; +return { props: /** @type {Record} */ ({}), exports: {}, bindings: "", slots: {}, events: {} }} +const Input__SvelteComponent_ = __sveltets_2_isomorphic_component(__sveltets_2_partial(__sveltets_2_with_any_event(render()))); +/*Ωignore_startΩ*/type Input__SvelteComponent_ = InstanceType; +/*Ωignore_endΩ*/export default Input__SvelteComponent_; \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/input.svelte new file mode 100644 index 000000000..e1c049400 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-4.v5/input.svelte @@ -0,0 +1,23 @@ + + + + +{#snippet chain()} +
{foo}
+{/snippet} + +{#snippet chain2()} + {@render chain()} +{/snippet} + +{#snippet chain3()} + {@render chain2()} +{/snippet} + +{#snippet hoistable()} +

hoist me

+{/snippet} \ No newline at end of file