Skip to content

feat: allow snippets to be exported from module scripts #14315

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

Merged
merged 26 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 22 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/famous-parents-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: allow snippets to be exported from module scripts
30 changes: 30 additions & 0 deletions documentation/docs/98-reference/.generated/compile-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,12 @@ Expected token %token%
Expected whitespace
```

### export_undefined

```
`%name%` is not defined
```

### global_reference_invalid

```
Expand Down Expand Up @@ -694,6 +700,30 @@ Cannot use `<slot>` syntax and `{@render ...}` tags in the same component. Migra
Cannot use explicit children snippet at the same time as implicit children content. Remove either the non-whitespace content or the children snippet block
```

### snippet_invalid_export

```
An exported snippet can only reference things declared in a `<script module>`, or other exportable snippets
```

It's possible to export a snippet from a `<script module>` block, but only if it doesn't reference anything defined inside a non-module-level `<script>`. For example you can't do this...

```svelte
<script module>
export { greeting };
</script>

<script>
let message = 'hello';
</script>

{#snippet greeting(name)}
<p>{message} {name}!</p>
{/snippet}
```

...because `greeting` references `message`, which is defined in the second `<script>`.

### snippet_invalid_rest_parameter

```
Expand Down
26 changes: 26 additions & 0 deletions packages/svelte/messages/compile-errors/script.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@

> `$effect()` can only be used as an expression statement

## export_undefined

> `%name%` is not defined

## global_reference_invalid

> `%name%` is an illegal variable name. To reference a global variable called `%name%`, use `globalThis.%name%`
Expand Down Expand Up @@ -134,6 +138,28 @@

> %name% cannot be used in runes mode

## snippet_invalid_export

> An exported snippet can only reference things declared in a `<script module>`, or other exportable snippets

It's possible to export a snippet from a `<script module>` block, but only if it doesn't reference anything defined inside a non-module-level `<script>`. For example you can't do this...

```svelte
<script module>
export { greeting };
</script>

<script>
let message = 'hello';
</script>

{#snippet greeting(name)}
<p>{message} {name}!</p>
{/snippet}
```

...because `greeting` references `message`, which is defined in the second `<script>`.

## snippet_parameter_assignment

> Cannot reassign or bind to snippet parameter
Expand Down
19 changes: 19 additions & 0 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ export function effect_invalid_placement(node) {
e(node, "effect_invalid_placement", "`$effect()` can only be used as an expression statement");
}

/**
* `%name%` is not defined
* @param {null | number | NodeLike} node
* @param {string} name
* @returns {never}
*/
export function export_undefined(node, name) {
e(node, "export_undefined", `\`${name}\` is not defined`);
}

/**
* `%name%` is an illegal variable name. To reference a global variable called `%name%`, use `globalThis.%name%`
* @param {null | number | NodeLike} node
Expand Down Expand Up @@ -395,6 +405,15 @@ export function runes_mode_invalid_import(node, name) {
e(node, "runes_mode_invalid_import", `${name} cannot be used in runes mode`);
}

/**
* An exported snippet can only reference things declared in a `<script module>`, or other exportable snippets
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function snippet_invalid_export(node) {
e(node, "snippet_invalid_export", "An exported snippet can only reference things declared in a `<script module>`, or other exportable snippets");
}

/**
* Cannot reassign or bind to snippet parameter
* @param {null | number | NodeLike} node
Expand Down
42 changes: 34 additions & 8 deletions packages/svelte/src/compiler/phases/1-parse/acorn.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,43 @@ const ParserWithTS = acorn.Parser.extend(tsPlugin({ allowSatisfies: true }));
/**
* @param {string} source
* @param {boolean} typescript
* @param {boolean} [is_script]
*/
export function parse(source, typescript) {
export function parse(source, typescript, is_script) {
const parser = typescript ? ParserWithTS : acorn.Parser;
const { onComment, add_comments } = get_comment_handlers(source);

const ast = parser.parse(source, {
onComment,
sourceType: 'module',
ecmaVersion: 13,
locations: true
});
// @ts-ignore
const parse_statement = parser.prototype.parseStatement;

// If we're dealing with a <script> then it might contain an export
// for something that doesn't exist directly inside but is inside the
// component instead, so we need to ensure that Acorn doesn't throw
// an error in these cases
if (is_script) {
// @ts-ignore
parser.prototype.parseStatement = function (...args) {
const v = parse_statement.call(this, ...args);
// @ts-ignore
this.undefinedExports = {};
return v;
};
}

let ast;

try {
ast = parser.parse(source, {
onComment,
sourceType: 'module',
ecmaVersion: 13,
locations: true
});
} finally {
if (is_script) {
// @ts-ignore
parser.prototype.parseStatement = parse_statement;
}
}

if (typescript) amend(source, ast);
add_comments(ast);
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/phases/1-parse/read/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function read_script(parser, start, attributes) {
let ast;

try {
ast = acorn.parse(source, parser.ts);
ast = acorn.parse(source, parser.ts, true);
} catch (err) {
parser.acorn_error(err);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/state/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,10 @@ function open(parser) {
name
},
parameters: function_expression.params,
body: create_fragment()
body: create_fragment(),
metadata: {
can_hoist: false
}
});
parser.stack.push(block);
parser.fragments.push(block.body);
Expand Down
20 changes: 16 additions & 4 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ export function analyze_component(root, source, options) {

const store_name = name.slice(1);
const declaration = instance.scope.get(store_name);
const init = /** @type {Node | undefined} */ (declaration?.initial);

// If we're not in legacy mode through the compiler option, assume the user
// is referencing a rune and not a global store.
Expand All @@ -295,9 +296,9 @@ export function analyze_component(root, source, options) {
!is_rune(name) ||
(declaration !== null &&
// const state = $state(0) is valid
(get_rune(declaration.initial, instance.scope) === null ||
(get_rune(init, instance.scope) === null ||
// rune-line names received as props are valid too (but we have to protect against $props as store)
(store_name !== 'props' && get_rune(declaration.initial, instance.scope) === '$props')) &&
(store_name !== 'props' && get_rune(init, instance.scope) === '$props')) &&
// allow `import { derived } from 'svelte/store'` in the same file as `const x = $derived(..)` because one is not a subscription to the other
!(
name === '$derived' &&
Expand Down Expand Up @@ -424,7 +425,6 @@ export function analyze_component(root, source, options) {
reactive_statements: new Map(),
binding_groups: new Map(),
slot_names: new Map(),
top_level_snippets: [],
css: {
ast: root.css,
hash: root.css
Expand All @@ -437,7 +437,8 @@ export function analyze_component(root, source, options) {
: '',
keyframes: []
},
source
source,
undefined_exports: new Map()
};

if (!runes) {
Expand Down Expand Up @@ -690,6 +691,17 @@ export function analyze_component(root, source, options) {
analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements);
}

for (const node of analysis.module.ast.body) {
if (node.type === 'ExportNamedDeclaration' && node.specifiers !== null) {
for (const specifier of node.specifiers) {
if (specifier.local.type !== 'Identifier') continue;

const binding = analysis.module.scope.get(specifier.local.name);
if (!binding) e.export_undefined(specifier, specifier.local.name);
}
}
}

if (analysis.event_directive_node && analysis.uses_event_attributes) {
e.mixed_event_handler_syntaxes(
analysis.event_directive_node,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/** @import { AST } from '#compiler' */
/** @import { AST, Binding, SvelteNode } from '#compiler' */
/** @import { Scope } from '../../scope' */
/** @import { Context } from '../types' */
import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js';
import * as e from '../../../errors.js';
Expand All @@ -22,6 +23,25 @@ export function SnippetBlock(node, context) {

context.next({ ...context.state, parent_element: null });

const can_hoist =
context.path.length === 1 &&
context.path[0].type === 'Fragment' &&
can_hoist_snippet(context.state.scope, context.state.scopes);

const name = node.expression.name;

if (can_hoist) {
const binding = /** @type {Binding} */ (context.state.scope.get(name));
context.state.analysis.module.scope.declarations.set(name, binding);
} else {
const undefined_export = context.state.analysis.undefined_exports.get(name);
if (undefined_export) {
e.snippet_invalid_export(undefined_export);
}
}

node.metadata.can_hoist = can_hoist;

const { path } = context;
const parent = path.at(-2);
if (!parent) return;
Expand Down Expand Up @@ -56,3 +76,35 @@ export function SnippetBlock(node, context) {
}
}
}

/**
* @param {Map<SvelteNode, Scope>} scopes
* @param {Scope} scope
*/
function can_hoist_snippet(scope, scopes, visited = new Set()) {
for (const [reference] of scope.references) {
const binding = scope.get(reference);

if (!binding || binding.scope.function_depth === 0) {
continue;
}

// ignore bindings declared inside the snippet (e.g. the snippet's own parameters)
if (binding.scope.function_depth >= scope.function_depth) {
continue;
}

if (binding.initial?.type === 'SnippetBlock') {
if (visited.has(binding)) continue;
visited.add(binding);

if (can_hoist_snippet(binding.scope, scopes, visited)) {
continue;
}
}

return false;
}

return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ export function client_component(analysis, options) {
private_state: new Map(),
transform: {},
in_constructor: false,
instance_level_snippets: [],
module_level_snippets: [],

// these are set inside the `Fragment` visitor, and cannot be used until then
before_init: /** @type {any} */ (null),
Expand Down Expand Up @@ -368,7 +370,7 @@ export function client_component(analysis, options) {
...store_setup,
...legacy_reactive_declarations,
...group_binding_declarations,
...analysis.top_level_snippets,
...state.instance_level_snippets,
.../** @type {ESTree.Statement[]} */ (instance.body),
analysis.runes || !analysis.needs_context
? b.empty
Expand Down Expand Up @@ -483,7 +485,7 @@ export function client_component(analysis, options) {
}
}

body = [...imports, ...body];
body = [...imports, ...state.module_level_snippets, ...body];

const component = b.function_declaration(
b.id(analysis.name),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import type {
PrivateIdentifier,
Expression,
AssignmentExpression,
UpdateExpression
UpdateExpression,
VariableDeclaration
} from 'estree';
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
import type { TransformState } from '../types.js';
Expand Down Expand Up @@ -85,6 +86,11 @@ export interface ComponentClientTransformState extends ClientTransformState {

/** The $: calls, which will be ordered in the end */
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;

/** Snippets hoisted to the instance */
readonly instance_level_snippets: VariableDeclaration[];
/** Snippets hoisted to the module */
readonly module_level_snippets: VariableDeclaration[];
}

export interface StateField {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ export function should_proxy(node, scope) {
binding.initial.type !== 'FunctionDeclaration' &&
binding.initial.type !== 'ClassDeclaration' &&
binding.initial.type !== 'ImportDeclaration' &&
binding.initial.type !== 'EachBlock'
binding.initial.type !== 'EachBlock' &&
binding.initial.type !== 'SnippetBlock'
) {
return should_proxy(binding.initial, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ export function SnippetBlock(node, context) {

// Top-level snippets are hoisted so they can be referenced in the `<script>`
if (context.path.length === 1 && context.path[0].type === 'Fragment') {
context.state.analysis.top_level_snippets.push(declaration);
if (node.metadata.can_hoist) {
context.state.module_level_snippets.push(declaration);
} else {
context.state.instance_level_snippets.push(declaration);
}
} else {
context.state.init.push(declaration);
}
Expand Down
Loading
Loading