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.

Sign up for GitHub

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: better hoistability analysis #2655

Merged
merged 1 commit into from
Jan 8, 2025
Merged

fix: better hoistability analysis #2655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions packages/svelte2tsx/src/svelte2tsx/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,13 @@ export function svelte2tsx(
}

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)));
(globals.size === 0 ||
[...globals.keys()].every((id) =>
exportedNames.hoistableInterfaces.isAllowedReference(id)
));

if (hoist_to_module) {
str.move(
Expand Down
122 changes: 46 additions & 76 deletions packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import MagicString from 'magic-string';
* Collects all imports and module-level declarations to then find out which interfaces/types are hoistable.
*/
export class HoistableInterfaces {
private import_value_set: Set<string> = new Set();
private import_type_set: Set<string> = new Set();
private module_types: Set<string> = new Set();
private disallowed_types = new Set<string>();
private disallowed_values = new Set<string>();
private interface_map: Map<
string,
{ type_deps: Set<string>; value_deps: Set<string>; node: ts.Node }
Expand All @@ -18,6 +19,7 @@ export class HoistableInterfaces {
value_deps: new Set<string>()
};

/** should be called before analyzeInstanceScriptNode */
analyzeModuleScriptNode(node: ts.Node) {
// Handle Import Declarations
if (ts.isImportDeclaration(node) && node.importClause) {
Expand All @@ -30,9 +32,7 @@ export class HoistableInterfaces {
node.importClause.namedBindings.elements.forEach((element) => {
const import_name = element.name.text;
if (is_type_only || element.isTypeOnly) {
this.import_type_set.add(import_name);
} else {
this.import_value_set.add(import_name);
this.module_types.add(import_name);
}
});
}
Expand All @@ -41,9 +41,7 @@ export class HoistableInterfaces {
if (node.importClause.name) {
const default_import = node.importClause.name.text;
if (is_type_only) {
this.import_type_set.add(default_import);
} else {
this.import_value_set.add(default_import);
this.module_types.add(default_import);
}
}

Expand All @@ -54,40 +52,17 @@ export class HoistableInterfaces {
) {
const namespace_import = node.importClause.namedBindings.name.text;
if (is_type_only) {
this.import_type_set.add(namespace_import);
} else {
this.import_value_set.add(namespace_import);
this.module_types.add(namespace_import);
}
}
}

// Handle top-level declarations
if (ts.isVariableStatement(node)) {
node.declarationList.declarations.forEach((declaration) => {
if (ts.isIdentifier(declaration.name)) {
this.import_value_set.add(declaration.name.text);
}
});
}

if (ts.isFunctionDeclaration(node) && node.name) {
this.import_value_set.add(node.name.text);
}

if (ts.isClassDeclaration(node) && node.name) {
this.import_value_set.add(node.name.text);
}

if (ts.isEnumDeclaration(node)) {
this.import_value_set.add(node.name.text);
}

if (ts.isTypeAliasDeclaration(node)) {
this.import_type_set.add(node.name.text);
this.module_types.add(node.name.text);
}

if (ts.isInterfaceDeclaration(node)) {
this.import_type_set.add(node.name.text);
this.module_types.add(node.name.text);
}
}

Expand All @@ -103,9 +78,7 @@ export class HoistableInterfaces {
node.importClause.namedBindings.elements.forEach((element) => {
const import_name = element.name.text;
if (is_type_only) {
this.import_type_set.add(import_name);
} else {
this.import_value_set.add(import_name);
this.module_types.add(import_name);
}
});
}
Expand All @@ -114,9 +87,7 @@ export class HoistableInterfaces {
if (node.importClause.name) {
const default_import = node.importClause.name.text;
if (is_type_only) {
this.import_type_set.add(default_import);
} else {
this.import_value_set.add(default_import);
this.module_types.add(default_import);
}
}

Expand All @@ -127,9 +98,7 @@ export class HoistableInterfaces {
) {
const namespace_import = node.importClause.namedBindings.name.text;
if (is_type_only) {
this.import_type_set.add(namespace_import);
} else {
this.import_value_set.add(namespace_import);
this.module_types.add(namespace_import);
}
}
}
Expand Down Expand Up @@ -167,9 +136,9 @@ export class HoistableInterfaces {
}
});

if (this.import_type_set.has(interface_name)) {
// shadowed; delete because we can't hoist
this.import_type_set.delete(interface_name);
if (this.module_types.has(interface_name)) {
// shadowed; we can't hoist
this.disallowed_types.add(interface_name);
} else {
this.interface_map.set(interface_name, {
type_deps: type_dependencies,
Expand All @@ -193,9 +162,9 @@ export class HoistableInterfaces {
generics
);

if (this.import_type_set.has(alias_name)) {
// shadowed; delete because we can't hoist
this.import_type_set.delete(alias_name);
if (this.module_types.has(alias_name)) {
// shadowed; we can't hoist
this.disallowed_types.add(alias_name);
} else {
this.interface_map.set(alias_name, {
type_deps: type_dependencies,
Expand All @@ -209,29 +178,21 @@ export class HoistableInterfaces {
if (ts.isVariableStatement(node)) {
node.declarationList.declarations.forEach((declaration) => {
if (ts.isIdentifier(declaration.name)) {
this.import_value_set.delete(declaration.name.text);
this.disallowed_values.add(declaration.name.text);
}
});
}

if (ts.isFunctionDeclaration(node) && node.name) {
this.import_value_set.delete(node.name.text);
this.disallowed_values.add(node.name.text);
}

if (ts.isClassDeclaration(node) && node.name) {
this.import_value_set.delete(node.name.text);
this.disallowed_values.add(node.name.text);
}

if (ts.isEnumDeclaration(node)) {
this.import_value_set.delete(node.name.text);
}

if (ts.isTypeAliasDeclaration(node)) {
this.import_type_set.delete(node.name.text);
}

if (ts.isInterfaceDeclaration(node)) {
this.import_type_set.delete(node.name.text);
this.disallowed_values.add(node.name.text);
}
}

Expand Down Expand Up @@ -281,13 +242,26 @@ export class HoistableInterfaces {
continue;
}

const can_hoist = [...deps.type_deps, ...deps.value_deps].every((dep) => {
return (
this.import_type_set.has(dep) ||
this.import_value_set.has(dep) ||
hoistable_interfaces.has(dep)
);
});
let can_hoist = true;

for (const dep of deps.type_deps) {
if (this.disallowed_types.has(dep)) {
this.disallowed_types.add(interface_name);
can_hoist = false;
break;
}
if (this.interface_map.has(dep) && !hoistable_interfaces.has(dep)) {
can_hoist = false;
}
}

for (const dep of deps.value_deps) {
if (this.disallowed_values.has(dep)) {
this.disallowed_types.add(interface_name);
can_hoist = false;
break;
}
}

if (can_hoist) {
hoistable_interfaces.set(interface_name, deps.node);
Expand All @@ -301,11 +275,7 @@ export class HoistableInterfaces {
...this.props_interface.type_deps,
...this.props_interface.value_deps
].every((dep) => {
return (
this.import_type_set.has(dep) ||
this.import_value_set.has(dep) ||
hoistable_interfaces.has(dep)
);
return !this.disallowed_types.has(dep) && !this.disallowed_values.has(dep);
});

if (can_hoist) {
Expand All @@ -328,7 +298,7 @@ export class HoistableInterfaces {
if (!this.props_interface.name) return;

for (const generic of generics) {
this.import_type_set.delete(generic);
this.disallowed_types.add(generic);
}

const hoistable = this.determineHoistableInterfaces();
Expand Down Expand Up @@ -362,8 +332,8 @@ export class HoistableInterfaces {
}
}

getAllowedValues() {
return this.import_value_set;
isAllowedReference(reference: string) {
return !this.disallowed_values.has(reference);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-1.v5/expectedv2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ import { imported } from './x';
{ 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)}; const hoistable8/*Ωignore_positionΩ*/ = ()/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
{ svelteHTML.createElement("div", {});global; }
};return __sveltets_2_any(0)}; const hoistable9/*Ωignore_positionΩ*/ = (props: HTMLAttributes<HTMLDivElement>)/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => { };return __sveltets_2_any(0)}; const hoistable10/*Ωignore_positionΩ*/ = (foo)/*Ωignore_startΩ*/: ReturnType<import('svelte').Snippet>/*Ωignore_endΩ*/ => { async ()/*Ωignore_positionΩ*/ => {
const bar = foo;
bar;
};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; }
Expand All @@ -40,6 +45,12 @@ async () => {












Expand Down
13 changes: 13 additions & 0 deletions packages/svelte2tsx/test/svelte2tsx/samples/snippet-module-hoist-1.v5/input.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@
<div>{imported}</div>
{/snippet}

{#snippet hoistable8()}
<div>{global}</div>
{/snippet}

{#snippet hoistable9(props: HTMLAttributes<HTMLDivElement>)}
Referencing global types
{/snippet}

{#snippet hoistable10(foo)}
{@const bar = foo}
{bar}
{/snippet}

{#snippet not_hoistable()}
<div>{foo}</div>
{/snippet}
Loading