Skip to content

Commit 57a7b0c

Browse files
Rich-HarrisRich-Harris
authored
chore: transformers (#12780)
* clear out getters on new scope * fix * fix * fix * fix * consolidate legacy_reactive_import logic * unused * $$sanitized_props * use getters mechanism for store_sub * snapshot * fix * tests passing * remove some stuff * more * fix * tidy up * simplify * simplify * getters -> transformers * update * use update transformers * add assign transformer * more * tweak * remove junk * unused * simplify * tidy up * tweak * assign_property * fix * tidy up * tidy up * move store code * this appears to be unused * tidy up * tweak * simplify * move code * move stuff * note to self * move stuff * each blocks * note to self * lengthen stack trace * tweak * more * tidy up * tidy up * remove some junk * tidy up * move stuff * remove stuff * tweak * tweak * fix * tweak * tidy up * tidy up * tidy up * tweak * simplify * tidy up * simplify * tidy up * improve output * delete comments * more * unused * tidy up * tidy up * fix * move some stuff * tweak * tidy up * DRY * synchronise * DRY out * tidy up * tidy up * tidy up * add test that fails on main * snapshot test * changesets * lint * ugh
1 parent f13c722 commit 57a7b0c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+907
-850
lines changed

‎.changeset/nine-ants-invite.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: invalidate signals following ++/-- inside each block

‎.changeset/perfect-hairs-matter.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
feat: better code generation for destructuring assignments

‎packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,7 @@ function get_delegated_event(event_name, handler, context) {
187187
// Bail out if we reference anything from the EachBlock (for now) that mutates in non-runes mode,
188188
(((!context.state.analysis.runes && binding.kind === 'each') ||
189189
// or any normal not reactive bindings that are mutated.
190-
binding.kind === 'normal' ||
191-
// or any reactive imports (those are rewritten) (can only happen in legacy mode)
192-
binding.kind === 'legacy_reactive_import') &&
190+
binding.kind === 'normal') &&
193191
binding.mutated))
194192
) {
195193
return unhoisted;

‎packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** @import { EachBlock } from '#compiler' */
22
/** @import { Context } from '../types' */
3+
/** @import { Scope } from '../../scope' */
34
import * as e from '../../../errors.js';
45
import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js';
56

@@ -25,5 +26,13 @@ export function EachBlock(node, context) {
2526
node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index;
2627
}
2728

28-
context.next();
29+
// evaluate expression in parent scope
30+
context.visit(node.expression, {
31+
...context.state,
32+
scope: /** @type {Scope} */ (context.state.scope.parent)
33+
});
34+
35+
context.visit(node.body);
36+
if (node.key) context.visit(node.key);
37+
if (node.fallback) context.visit(node.fallback);
2938
}

‎packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,8 @@ export function Identifier(node, context) {
8080
binding.declaration_kind !== 'function') ||
8181
binding.declaration_kind === 'import')
8282
) {
83-
if (binding.declaration_kind === 'import') {
84-
if (
85-
binding.mutated &&
86-
// TODO could be more fine-grained - not every mention in the template implies a state binding
87-
(context.state.reactive_statement || context.state.ast_type === 'template') &&
88-
parent.type === 'MemberExpression'
89-
) {
90-
binding.kind = 'legacy_reactive_import';
91-
}
92-
} else if (
83+
if (
84+
binding.declaration_kind !== 'import' &&
9385
binding.mutated &&
9486
// TODO could be more fine-grained - not every mention in the template implies a state binding
9587
(context.state.reactive_statement || context.state.ast_type === 'template')

‎packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
/** @import { Component, SvelteComponent, SvelteSelf } from '#compiler' */
1+
/** @import { Comment, Component, Fragment, SvelteComponent, SvelteSelf } from '#compiler' */
22
/** @import { Context } from '../../types' */
33
import * as e from '../../../../errors.js';
44
import { get_attribute_expression, is_expression_attribute } from '../../../../utils/ast.js';
5+
import { determine_slot } from '../../../../utils/slot.js';
56
import {
67
validate_attribute,
78
validate_attribute_name,
@@ -60,9 +61,45 @@ export function visit_component(node, context) {
6061
}
6162
}
6263

63-
context.next({
64-
...context.state,
65-
parent_element: null,
66-
component_slots: new Set()
67-
});
64+
// If the component has a slot attribute — `<Foo slot="whatever" .../>` —
65+
// then `let:` directives apply to other attributes, instead of just the
66+
// top-level contents of the component. Yes, this is very weird.
67+
const default_state = determine_slot(node)
68+
? context.state
69+
: { ...context.state, scope: node.metadata.scopes.default };
70+
71+
for (const attribute of node.attributes) {
72+
context.visit(attribute, attribute.type === 'LetDirective' ? default_state : context.state);
73+
}
74+
75+
/** @type {Comment[]} */
76+
let comments = [];
77+
78+
/** @type {Record<string, Fragment['nodes']>} */
79+
const nodes = { default: [] };
80+
81+
for (const child of node.fragment.nodes) {
82+
if (child.type === 'Comment') {
83+
comments.push(child);
84+
continue;
85+
}
86+
87+
const slot_name = determine_slot(child) ?? 'default';
88+
(nodes[slot_name] ??= []).push(...comments, child);
89+
90+
if (slot_name !== 'default') comments = [];
91+
}
92+
93+
const component_slots = new Set();
94+
95+
for (const slot_name in nodes) {
96+
const state = {
97+
...context.state,
98+
scope: node.metadata.scopes[slot_name],
99+
parent_element: null,
100+
component_slots
101+
};
102+
103+
context.visit({ ...node.fragment, nodes: nodes[slot_name] }, state);
104+
}
68105
}

‎packages/svelte/src/compiler/phases/3-transform/client/transform-client.js

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
/** @import { Visitors, ComponentClientTransformState, ClientTransformState } from './types' */
55
import { walk } from 'zimmerframe';
66
import * as b from '../../../utils/builders.js';
7-
import { set_scope } from '../../scope.js';
87
import { build_getter } from './utils.js';
98
import { render_stylesheet } from '../css/index.js';
109
import { dev, filename } from '../../../state.js';
@@ -15,6 +14,7 @@ import { Attribute } from './visitors/Attribute.js';
1514
import { AwaitBlock } from './visitors/AwaitBlock.js';
1615
import { BinaryExpression } from './visitors/BinaryExpression.js';
1716
import { BindDirective } from './visitors/BindDirective.js';
17+
import { BlockStatement } from './visitors/BlockStatement.js';
1818
import { BreakStatement } from './visitors/BreakStatement.js';
1919
import { CallExpression } from './visitors/CallExpression.js';
2020
import { ClassBody } from './visitors/ClassBody.js';
@@ -37,6 +37,7 @@ import { LabeledStatement } from './visitors/LabeledStatement.js';
3737
import { LetDirective } from './visitors/LetDirective.js';
3838
import { MemberExpression } from './visitors/MemberExpression.js';
3939
import { OnDirective } from './visitors/OnDirective.js';
40+
import { Program } from './visitors/Program.js';
4041
import { RegularElement } from './visitors/RegularElement.js';
4142
import { RenderTag } from './visitors/RenderTag.js';
4243
import { SlotElement } from './visitors/SlotElement.js';
@@ -58,14 +59,31 @@ import { VariableDeclaration } from './visitors/VariableDeclaration.js';
5859

5960
/** @type {Visitors} */
6061
const visitors = {
61-
_: set_scope,
62+
_: function set_scope(node, { next, state }) {
63+
const scope = state.scopes.get(node);
64+
65+
if (scope && scope !== state.scope) {
66+
const transform = { ...state.transform };
67+
68+
for (const [name, binding] of scope.declarations) {
69+
if (binding.kind === 'normal') {
70+
delete transform[name];
71+
}
72+
}
73+
74+
next({ ...state, transform, scope });
75+
} else {
76+
next();
77+
}
78+
},
6279
AnimateDirective,
6380
ArrowFunctionExpression,
6481
AssignmentExpression,
6582
Attribute,
6683
AwaitBlock,
6784
BinaryExpression,
6885
BindDirective,
86+
BlockStatement,
6987
BreakStatement,
7088
CallExpression,
7189
ClassBody,
@@ -88,6 +106,7 @@ const visitors = {
88106
LetDirective,
89107
MemberExpression,
90108
OnDirective,
109+
Program,
91110
RegularElement,
92111
RenderTag,
93112
SlotElement,
@@ -123,6 +142,7 @@ export function client_component(analysis, options) {
123142
is_instance: false,
124143
hoisted: [b.import_all('$', 'svelte/internal/client')],
125144
node: /** @type {any} */ (null), // populated by the root node
145+
legacy_reactive_imports: [],
126146
legacy_reactive_statements: new Map(),
127147
metadata: {
128148
context: {
@@ -136,8 +156,7 @@ export function client_component(analysis, options) {
136156
preserve_whitespace: options.preserveWhitespace,
137157
public_state: new Map(),
138158
private_state: new Map(),
139-
getters: {},
140-
setters: {},
159+
transform: {},
141160
in_constructor: false,
142161

143162
// these are set inside the `Fragment` visitor, and cannot be used until then
@@ -155,6 +174,7 @@ export function client_component(analysis, options) {
155174

156175
const instance_state = {
157176
...state,
177+
transform: { ...state.transform },
158178
scope: analysis.instance.scope,
159179
scopes: analysis.instance.scopes,
160180
is_instance: true
@@ -167,21 +187,17 @@ export function client_component(analysis, options) {
167187
const template = /** @type {ESTree.Program} */ (
168188
walk(
169189
/** @type {SvelteNode} */ (analysis.template.ast),
170-
{ ...state, scope: analysis.instance.scope, scopes: analysis.template.scopes },
190+
{
191+
...state,
192+
transform: instance_state.transform,
193+
scope: analysis.instance.scope,
194+
scopes: analysis.template.scopes
195+
},
171196
visitors
172197
)
173198
);
174199

175-
// Very very dirty way of making import statements reactive in legacy mode if needed
176-
if (!analysis.runes) {
177-
for (const [name, binding] of analysis.module.scope.declarations) {
178-
if (binding.kind === 'legacy_reactive_import') {
179-
instance.body.unshift(
180-
b.var('$$_import_' + name, b.call('$.reactive_import', b.thunk(b.id(name))))
181-
);
182-
}
183-
}
184-
}
200+
instance.body.unshift(...state.legacy_reactive_imports);
185201

186202
/** @type {ESTree.Statement[]} */
187203
const store_setup = [];
@@ -623,11 +639,9 @@ export function client_module(analysis, options) {
623639
options,
624640
scope: analysis.module.scope,
625641
scopes: analysis.module.scopes,
626-
legacy_reactive_statements: new Map(),
627642
public_state: new Map(),
628643
private_state: new Map(),
629-
getters: {},
630-
setters: {},
644+
transform: {},
631645
in_constructor: false
632646
};
633647

‎packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import type {
55
Identifier,
66
PrivateIdentifier,
77
Expression,
8-
AssignmentExpression
8+
AssignmentExpression,
9+
UpdateExpression
910
} from 'estree';
1011
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
1112
import type { TransformState } from '../types.js';
@@ -22,19 +23,18 @@ export interface ClientTransformState extends TransformState {
2223
*/
2324
readonly in_constructor: boolean;
2425

25-
/** The $: calls, which will be ordered in the end */
26-
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
27-
/**
28-
* A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
29-
* will be replaced with `node` (e.g. `x` -> `$.get(x)`)
30-
*/
31-
readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
32-
/**
33-
* Counterpart to `getters`
34-
*/
35-
readonly setters: Record<
26+
readonly transform: Record<
3627
string,
37-
(assignment: AssignmentExpression, context: Context) => Expression
28+
{
29+
/** turn `foo` into e.g. `$.get(foo)` */
30+
read: (id: Identifier) => Expression;
31+
/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
32+
assign?: (node: Identifier, value: Expression) => Expression;
33+
/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */
34+
mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression;
35+
/** turn `foo++` into e.g. `$.update(foo)` */
36+
update?: (node: UpdateExpression) => Expression;
37+
}
3838
>;
3939
}
4040

@@ -79,6 +79,12 @@ export interface ComponentClientTransformState extends ClientTransformState {
7979

8080
/** The anchor node for the current context */
8181
readonly node: Identifier;
82+
83+
/** Imports that should be re-evaluated in legacy mode following a mutation */
84+
readonly legacy_reactive_imports: Statement[];
85+
86+
/** The $: calls, which will be ordered in the end */
87+
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
8288
}
8389

8490
export interface StateField {

0 commit comments

Comments
 (0)