Skip to content

Commit

Permalink
feat: hoist snippets to module context if possible (#2601)
Browse files Browse the repository at this point in the history
  • Loading branch information
dummdidumm authored Nov 21, 2024
1 parent 050ecc1 commit 9a5a6af
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 10 deletions.
21 changes: 18 additions & 3 deletions packages/svelte2tsx/src/htmlxtojsx_v2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
} from '../svelte2tsx/nodes/handleScopeAndResolveForSlot';
import { EventHandler } from '../svelte2tsx/nodes/event-handler';
import { ComponentEvents } from '../svelte2tsx/nodes/ComponentEvents';
import { analyze } from 'periscopic';

export interface TemplateProcessResult {
/**
Expand All @@ -53,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<[number, number]>;
rootSnippets: Array<[start: number, end: number, globals: Map<string, any>]>;
/** To be added later as a comment on the default class export */
componentDocumentation: ComponentDocumentation;
events: ComponentEvents;
Expand Down Expand Up @@ -92,7 +93,7 @@ export function convertHtmlxToJsx(

stripDoctype(str);

const rootSnippets: Array<[number, number]> = [];
const rootSnippets: Array<[number, number, Map<string, any>]> = [];
let element: Element | InlineComponent | undefined;

const pendingSnippetHoistCheck = new Set<BaseNode>();
Expand Down Expand Up @@ -249,7 +250,21 @@ export function convertHtmlxToJsx(
);
if (parent === ast) {
// root snippet -> move to instance script or possibly even module script
rootSnippets.push([node.start, node.end]);
const result = analyze({
type: 'FunctionDeclaration',
start: -1,
end: -1,
id: node.expression,
params: node.parameters ?? [],
body: {
type: 'BlockStatement',
start: -1,
end: -1,
body: node.children as any[] // wrong AST, but periscopic doesn't care
}
});

rootSnippets.push([node.start, node.end, result.globals]);
} else {
pendingSnippetHoistCheck.add(parent);
}
Expand Down
6 changes: 0 additions & 6 deletions packages/svelte2tsx/src/svelte2tsx/createRenderFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export interface CreateRenderFunctionPara extends InstanceScriptProcessResult {
str: MagicString;
scriptTag: Node;
scriptDestination: number;
rootSnippets: Array<[number, number]>;
slots: Map<string, Map<string, string>>;
events: ComponentEvents;
uses$$SlotsInterface: boolean;
Expand All @@ -20,7 +19,6 @@ export function createRenderFunction({
str,
scriptTag,
scriptDestination,
rootSnippets,
slots,
events,
exportedNames,
Expand Down Expand Up @@ -82,10 +80,6 @@ export function createRenderFunction({
);
}

for (const rootSnippet of rootSnippets) {
str.move(rootSnippet[0], rootSnippet[1], scriptTagEnd);
}

const scriptEndTagStart = htmlx.lastIndexOf('<', scriptTag.end - 1);
// wrap template with callback
str.overwrite(scriptEndTagStart, scriptTag.end, `${slotsDeclaration};\nasync () => {`, {
Expand Down
27 changes: 26 additions & 1 deletion packages/svelte2tsx/src/svelte2tsx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ export function svelte2tsx(
str,
scriptTag,
scriptDestination: instanceScriptTarget,
rootSnippets,
slots,
events,
exportedNames,
Expand All @@ -164,6 +163,32 @@ export function svelte2tsx(
),
moduleAst
);
if (!scriptTag) {
moduleAst.tsAst.forEachChild((node) =>
exportedNames.hoistableInterfaces.analyzeModuleScriptNode(node)
);
}
}

if (moduleScriptTag || scriptTag) {
const allowed = exportedNames.hoistableInterfaces.getAllowedValues();
for (const [start, end, globals] of rootSnippets) {
const hoist_to_module =
moduleScriptTag &&
(globals.size === 0 || [...globals.keys()].every((id) => allowed.has(id)));

if (hoist_to_module) {
str.move(
start,
end,
scriptTag
? scriptTag.start + 1 // +1 because imports are also moved at that position, and we want to move interfaces after imports
: moduleScriptTag.end
);
} else if (scriptTag) {
str.move(start, end, renderFunctionStart);
}
}
}

addComponentExport({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ export class HoistableInterfaces {
}
}

getAllowedValues() {
return this.import_value_set;
}

/**
* Collects type and value dependencies from a given TypeNode.
* @param type_node The TypeNode to analyze.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
///<reference types="svelte" />
;
let module = true;
;;

import { imported } from './x';
const hoistable1/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {}); }
};return __sveltets_2_any(0)}; const hoistable2/*Ωignore_positionΩ*/ = (bar)/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});bar; }
};return __sveltets_2_any(0)}; const hoistable3/*Ωignore_positionΩ*/ = (bar: string)/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});bar; }
};return __sveltets_2_any(0)}; const hoistable4/*Ωignore_positionΩ*/ = (foo)/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});foo; }
};return __sveltets_2_any(0)}; const hoistable5/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("button", { "onclick":e => e,}); }
};return __sveltets_2_any(0)}; const hoistable6/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});module; }
};return __sveltets_2_any(0)}; const hoistable7/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});imported; }
};return __sveltets_2_any(0)};function render() {
const not_hoistable/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});foo; }
};return __sveltets_2_any(0)};

let foo = true;
;
async () => {

















};
return { props: /** @type {Record<string, never>} */ ({}), 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<typeof Input__SvelteComponent_>;
/*Ωignore_endΩ*/export default Input__SvelteComponent_;
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<script module>
let module = true;
</script>

<script lang="ts">
import { imported } from './x';
let foo = true;
</script>

{#snippet hoistable1()}
<div>hello</div>
{/snippet}

{#snippet hoistable2(bar)}
<div>{bar}</div>
{/snippet}

{#snippet hoistable3(bar: string)}
<div>{bar}</div>
{/snippet}

{#snippet hoistable4(foo)}
<div>{foo}</div>
{/snippet}

{#snippet hoistable5()}
<button onclick={e => e}>click</button>
{/snippet}

{#snippet hoistable6()}
<div>{module}</div>
{/snippet}

{#snippet hoistable7()}
<div>{imported}</div>
{/snippet}

{#snippet not_hoistable()}
<div>{foo}</div>
{/snippet}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
///<reference types="svelte" />
;
import { imported } from './x';
function render() {
const hoistable/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {}); }
};return __sveltets_2_any(0)}; const not_hoistable/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});foo; }
};return __sveltets_2_any(0)};

let foo = true;
;
async () => {



};
return { props: /** @type {Record<string, never>} */ ({}), 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<typeof Input__SvelteComponent_>;
/*Ωignore_endΩ*/export default Input__SvelteComponent_;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script lang="ts">
import { imported } from './x';
let foo = true;
</script>

{#snippet hoistable()}
<div>hello</div>
{/snippet}

{#snippet not_hoistable()}
<div>{foo}</div>
{/snippet}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
///<reference types="svelte" />
;
let foo = true;
; const hoistable1/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {}); }
};return __sveltets_2_any(0)}; const hoistable2/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});foo; }
};return __sveltets_2_any(0)};;function render() {
async () => {



};
return { props: /** @type {Record<string, never>} */ ({}), 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<typeof Input__SvelteComponent_>;
/*Ωignore_endΩ*/export default Input__SvelteComponent_;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script module lang="ts">
let foo = true;
</script>

{#snippet hoistable1()}
<div>hello</div>
{/snippet}

{#snippet hoistable2()}
<div>{foo}</div>
{/snippet}

0 comments on commit 9a5a6af

Please sign in to comment.