Skip to content

Commit 19beb77

Browse files
Rich-HarrisRich-Harris
authored
chore: set binding.kind before analysis (#12843)
* analyse exports before walking * more * another * this is unused * move stuff/tidy up * this appears to be unnecessary * this is all unnecessary * simplify * simplify * simplify * simplify * move more stuff over * changeset * unused * separate reassignment from mutation * regenerate * lint
1 parent 32808ac commit 19beb77

File tree

15 files changed

+143
-152
lines changed

15 files changed

+143
-152
lines changed

‎.changeset/pink-countries-repair.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+
chore: set `binding.kind` before analysis

‎packages/svelte/src/compiler/migrate/index.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,7 @@ const instance_script = {
331331

332332
const binding = /** @type {Compiler.Binding} */ (state.scope.get(declarator.id.name));
333333

334-
if (
335-
state.analysis.uses_props &&
336-
(declarator.init || binding.mutated || binding.reassigned)
337-
) {
334+
if (state.analysis.uses_props && (declarator.init || binding.updated)) {
338335
throw new Error(
339336
'$$props is used together with named props in a way that cannot be automatically migrated.'
340337
);
@@ -350,7 +347,7 @@ const instance_script = {
350347
)
351348
: '',
352349
optional: !!declarator.init,
353-
bindable: binding.mutated || binding.reassigned,
350+
bindable: binding.updated,
354351
...extract_type_and_comment(declarator, state.str, path)
355352
});
356353
state.props_insertion_point = /** @type {number} */ (declarator.end);

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

Lines changed: 111 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
/** @import { Node, Program } from 'estree' */
2-
/** @import { Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */
1+
/** @import { Expression, Node, Program } from 'estree' */
2+
/** @import { Binding, Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */
33
/** @import { AnalysisState, Visitors } from './types' */
44
/** @import { Analysis, ComponentAnalysis, Js, ReactiveStatement, Template } from '../types' */
55
import { walk } from 'zimmerframe';
66
import * as e from '../../errors.js';
77
import * as w from '../../warnings.js';
8-
import { is_text_attribute } from '../../utils/ast.js';
8+
import { extract_identifiers, is_text_attribute } from '../../utils/ast.js';
99
import * as b from '../../utils/builders.js';
10-
import { Scope, ScopeRoot, create_scopes, get_rune } from '../scope.js';
10+
import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js';
1111
import check_graph_for_cycles from './utils/check_graph_for_cycles.js';
1212
import { create_attribute } from '../nodes.js';
1313
import { analyze_css } from './css/css-analyze.js';
@@ -62,6 +62,7 @@ import { Text } from './visitors/Text.js';
6262
import { TitleElement } from './visitors/TitleElement.js';
6363
import { UpdateExpression } from './visitors/UpdateExpression.js';
6464
import { VariableDeclarator } from './visitors/VariableDeclarator.js';
65+
import is_reference from 'is-reference';
6566

6667
/**
6768
* @type {Visitors}
@@ -397,6 +398,112 @@ export function analyze_component(root, source, options) {
397398
source
398399
};
399400

401+
if (!runes) {
402+
// every exported `let` or `var` declaration becomes a prop, everything else becomes an export
403+
for (const node of instance.ast.body) {
404+
if (node.type !== 'ExportNamedDeclaration') continue;
405+
406+
analysis.needs_props = true;
407+
408+
if (node.declaration) {
409+
if (
410+
node.declaration.type === 'FunctionDeclaration' ||
411+
node.declaration.type === 'ClassDeclaration'
412+
) {
413+
analysis.exports.push({
414+
name: /** @type {import('estree').Identifier} */ (node.declaration.id).name,
415+
alias: null
416+
});
417+
} else if (node.declaration.type === 'VariableDeclaration') {
418+
if (node.declaration.kind === 'const') {
419+
for (const declarator of node.declaration.declarations) {
420+
for (const node of extract_identifiers(declarator.id)) {
421+
analysis.exports.push({ name: node.name, alias: null });
422+
}
423+
}
424+
} else {
425+
for (const declarator of node.declaration.declarations) {
426+
for (const id of extract_identifiers(declarator.id)) {
427+
const binding = /** @type {Binding} */ (instance.scope.get(id.name));
428+
binding.kind = 'bindable_prop';
429+
}
430+
}
431+
}
432+
}
433+
} else {
434+
for (const specifier of node.specifiers) {
435+
const binding = instance.scope.get(specifier.local.name);
436+
437+
if (
438+
binding &&
439+
(binding.declaration_kind === 'var' || binding.declaration_kind === 'let')
440+
) {
441+
binding.kind = 'bindable_prop';
442+
443+
if (specifier.exported.name !== specifier.local.name) {
444+
binding.prop_alias = specifier.exported.name;
445+
}
446+
} else {
447+
analysis.exports.push({ name: specifier.local.name, alias: specifier.exported.name });
448+
}
449+
}
450+
}
451+
}
452+
453+
// if reassigned/mutated bindings are referenced in `$:` blocks
454+
// or the template, turn them into state
455+
for (const binding of instance.scope.declarations.values()) {
456+
if (binding.kind !== 'normal') continue;
457+
458+
for (const { node, path } of binding.references) {
459+
if (node === binding.node) continue;
460+
461+
if (binding.updated) {
462+
if (
463+
path[path.length - 1].type === 'StyleDirective' ||
464+
path.some((node) => node.type === 'Fragment') ||
465+
(path[1].type === 'LabeledStatement' && path[1].label.name === '$')
466+
) {
467+
binding.kind = 'state';
468+
}
469+
}
470+
}
471+
}
472+
473+
// more legacy nonsense: if an `each` binding is reassigned/mutated,
474+
// treat the expression as being mutated as well
475+
walk(/** @type {SvelteNode} */ (template.ast), null, {
476+
EachBlock(node) {
477+
const scope = /** @type {Scope} */ (template.scopes.get(node));
478+
479+
for (const binding of scope.declarations.values()) {
480+
if (binding.updated) {
481+
const state = { scope: /** @type {Scope} */ (scope.parent), scopes: template.scopes };
482+
483+
walk(node.expression, state, {
484+
// @ts-expect-error
485+
_: set_scope,
486+
Identifier(node, context) {
487+
const parent = /** @type {Expression} */ (context.path.at(-1));
488+
489+
if (is_reference(node, parent)) {
490+
const binding = context.state.scope.get(node.name);
491+
492+
if (binding && binding.kind === 'normal') {
493+
binding.kind = 'state';
494+
binding.mutated = binding.updated = true;
495+
}
496+
}
497+
}
498+
});
499+
500+
break;
501+
}
502+
}
503+
}
504+
});
505+
}
506+
400507
if (root.options) {
401508
for (const attribute of root.options.attributes) {
402509
if (attribute.name === 'accessors') {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ function get_delegated_event(event_name, handler, context) {
134134
return unhoisted;
135135
}
136136

137-
if (binding !== null && binding.initial !== null && !binding.mutated && !binding.is_called) {
137+
if (binding !== null && binding.initial !== null && !binding.updated && !binding.is_called) {
138138
const binding_type = binding.initial.type;
139139

140140
if (
@@ -188,7 +188,7 @@ function get_delegated_event(event_name, handler, context) {
188188
(((!context.state.analysis.runes && binding.kind === 'each') ||
189189
// or any normal not reactive bindings that are mutated.
190190
binding.kind === 'normal') &&
191-
binding.mutated))
191+
binding.updated))
192192
) {
193193
return unhoisted;
194194
}

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function BindDirective(node, context) {
3939
binding.kind !== 'bindable_prop' &&
4040
binding.kind !== 'each' &&
4141
binding.kind !== 'store_sub' &&
42-
!binding.mutated))
42+
!binding.updated)) // TODO wut?
4343
) {
4444
e.bind_invalid_value(node.expression);
4545
}
@@ -80,10 +80,6 @@ export function BindDirective(node, context) {
8080
if (references.length > 0) {
8181
parent.metadata.contains_group_binding = true;
8282

83-
for (const binding of parent.metadata.references) {
84-
binding.mutated = true;
85-
}
86-
8783
each_blocks.push(parent);
8884
ids = ids.filter((id) => !references.includes(id));
8985
ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]);

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

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -39,37 +39,6 @@ export function ExportNamedDeclaration(node, context) {
3939
}
4040
}
4141

42-
if (context.state.ast_type === 'instance' && !context.state.analysis.runes) {
43-
context.state.analysis.needs_props = true;
44-
45-
if (node.declaration) {
46-
if (
47-
node.declaration.type === 'FunctionDeclaration' ||
48-
node.declaration.type === 'ClassDeclaration'
49-
) {
50-
context.state.analysis.exports.push({
51-
name: /** @type {Identifier} */ (node.declaration.id).name,
52-
alias: null
53-
});
54-
} else if (node.declaration.type === 'VariableDeclaration') {
55-
if (node.declaration.kind === 'const') {
56-
for (const declarator of node.declaration.declarations) {
57-
for (const node of extract_identifiers(declarator.id)) {
58-
context.state.analysis.exports.push({ name: node.name, alias: null });
59-
}
60-
}
61-
} else {
62-
for (const declarator of node.declaration.declarations) {
63-
for (const id of extract_identifiers(declarator.id)) {
64-
const binding = /** @type {Binding} */ (context.state.scope.get(id.name));
65-
binding.kind = 'bindable_prop';
66-
}
67-
}
68-
}
69-
}
70-
}
71-
}
72-
7342
if (context.state.analysis.runes) {
7443
if (node.declaration && context.state.ast_type === 'instance') {
7544
if (

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

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,7 @@ export function ExportSpecifier(node, context) {
1717
});
1818

1919
const binding = context.state.scope.get(node.local.name);
20-
if (binding) binding.reassigned = true;
21-
} else {
22-
context.state.analysis.needs_props = true;
23-
24-
const binding = /** @type {Binding} */ (context.state.scope.get(node.local.name));
25-
26-
if (
27-
binding !== null &&
28-
(binding.kind === 'state' ||
29-
binding.kind === 'raw_state' ||
30-
(binding.kind === 'normal' &&
31-
(binding.declaration_kind === 'let' || binding.declaration_kind === 'var')))
32-
) {
33-
binding.kind = 'bindable_prop';
34-
if (node.exported.name !== node.local.name) {
35-
binding.prop_alias = node.exported.name;
36-
}
37-
} else {
38-
context.state.analysis.exports.push({
39-
name: node.local.name,
40-
alias: node.exported.name
41-
});
42-
}
20+
if (binding) binding.reassigned = binding.updated = true;
4321
}
4422
} else {
4523
validate_export(node, context.state.scope, node.local.name);

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

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/** @import { Expression, Identifier } from 'estree' */
2+
/** @import { EachBlock } from '#compiler' */
23
/** @import { Context } from '../types' */
34
import is_reference from 'is-reference';
45
import { should_proxy } from '../../3-transform/client/utils.js';
@@ -77,46 +78,6 @@ export function Identifier(node, context) {
7778
if (node.name === '$$restProps') {
7879
context.state.analysis.uses_rest_props = true;
7980
}
80-
81-
if (
82-
binding?.kind === 'normal' &&
83-
((binding.scope === context.state.instance_scope &&
84-
binding.declaration_kind !== 'function') ||
85-
binding.declaration_kind === 'import')
86-
) {
87-
if (
88-
binding.declaration_kind !== 'import' &&
89-
binding.mutated &&
90-
// TODO could be more fine-grained - not every mention in the template implies a state binding
91-
(context.state.reactive_statement || context.state.ast_type === 'template')
92-
) {
93-
binding.kind = 'state';
94-
} else if (
95-
context.state.reactive_statement &&
96-
parent.type === 'AssignmentExpression' &&
97-
parent.left === binding.node
98-
) {
99-
binding.kind = 'derived';
100-
}
101-
} else if (binding?.kind === 'each' && binding.mutated) {
102-
// Ensure that the array is marked as reactive even when only its entries are mutated
103-
let i = context.path.length;
104-
while (i--) {
105-
const ancestor = context.path[i];
106-
if (
107-
ancestor.type === 'EachBlock' &&
108-
context.state.analysis.template.scopes.get(ancestor)?.declarations.get(node.name) ===
109-
binding
110-
) {
111-
for (const binding of ancestor.metadata.references) {
112-
if (binding.kind === 'normal') {
113-
binding.kind = 'state';
114-
}
115-
}
116-
break;
117-
}
118-
}
119-
}
12081
}
12182

12283
if (binding) {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ export function StyleDirective(node, context) {
1717
let binding = context.state.scope.get(node.name);
1818

1919
if (binding) {
20-
if (!context.state.analysis.runes && binding.mutated) {
21-
binding.kind = 'state';
22-
}
23-
2420
if (binding.kind !== 'normal') {
2521
node.metadata.expression.has_state = true;
2622
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ export function is_prop_source(binding, state) {
235235
binding.initial ||
236236
// Until legacy mode is gone, we also need to use the prop source when only mutated is true,
237237
// because the parent could be a legacy component which needs coarse-grained reactivity
238-
binding.mutated)
238+
binding.updated)
239239
);
240240
}
241241

0 commit comments

Comments
 (0)