From 153709460a1d142124a452783b104e16e932a23e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 16 Aug 2024 16:42:10 +0100 Subject: [PATCH 1/7] chore: inline start and end node properties into effect --- .changeset/eighty-bugs-fetch.md | 5 +++++ .../svelte/src/internal/client/dom/blocks/each.js | 10 +++++----- .../internal/client/dom/blocks/svelte-element.js | 4 ++-- .../svelte/src/internal/client/dom/template.js | 9 ++++++--- .../src/internal/client/reactivity/effects.js | 14 ++++++++------ .../src/internal/client/reactivity/types.d.ts | 8 ++------ packages/svelte/src/internal/client/render.js | 4 ++-- packages/svelte/src/internal/client/runtime.js | 2 +- 8 files changed, 31 insertions(+), 25 deletions(-) create mode 100644 .changeset/eighty-bugs-fetch.md diff --git a/.changeset/eighty-bugs-fetch.md b/.changeset/eighty-bugs-fetch.md new file mode 100644 index 000000000000..35bde6a377a6 --- /dev/null +++ b/.changeset/eighty-bugs-fetch.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: inline start and end node properties into effect diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 18785f711483..85500de7e0d4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -1,4 +1,4 @@ -/** @import { EachItem, EachState, Effect, EffectNodes, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */ +/** @import { EachItem, EachState, Effect, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */ import { EACH_INDEX_REACTIVE, EACH_IS_ANIMATED, @@ -288,7 +288,7 @@ function reconcile(array, state, anchor, render_fn, flags, get_key) { item = items.get(key); if (item === undefined) { - var child_anchor = current ? /** @type {EffectNodes} */ (current.e.nodes).start : anchor; + var child_anchor = current ? /** @type {TemplateNode} */ (current.e.n_start) : anchor; prev = create_item( child_anchor, @@ -509,10 +509,10 @@ function create_item(anchor, state, prev, next, value, key, index, render_fn, fl * @param {Text | Element | Comment} anchor */ function move(item, next, anchor) { - var end = item.next ? /** @type {EffectNodes} */ (item.next.e.nodes).start : anchor; + var end = item.next ? /** @type {TemplateNode} */ (item.next.e.n_start) : anchor; - var dest = next ? /** @type {EffectNodes} */ (next.e.nodes).start : anchor; - var node = /** @type {EffectNodes} */ (item.e.nodes).start; + var dest = next ? /** @type {TemplateNode} */ (next.e.n_start) : anchor; + var node = /** @type {TemplateNode} */ (item.e.n_start); while (node !== end) { var next_node = /** @type {TemplateNode} */ (get_next_sibling(node)); diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index 755e44c9ac70..c602b274d550 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -1,4 +1,4 @@ -/** @import { Effect, EffectNodes, TemplateNode } from '#client' */ +/** @import { Effect, TemplateNode } from '#client' */ import { FILENAME, NAMESPACE_SVG } from '../../../../constants.js'; import { hydrate_next, @@ -138,7 +138,7 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio } // we do this after calling `render_fn` so that child effects don't override `nodes.end` - /** @type {Effect & { nodes: EffectNodes }} */ (current_effect).nodes.end = element; + /** @type {Effect} */ (current_effect).n_end = element; anchor.before(element); }); diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 27d82bb0a495..8bd0dbb72544 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -1,4 +1,4 @@ -/** @import { Effect, EffectNodes, TemplateNode } from '#client' */ +/** @import { Effect, TemplateNode } from '#client' */ import { hydrate_next, hydrate_node, hydrating, set_hydrate_node } from './hydration.js'; import { create_text, get_first_child } from './operations.js'; import { create_fragment_from_html } from './reconciler.js'; @@ -11,7 +11,10 @@ import { queue_micro_task } from './task.js'; * @param {TemplateNode | null} end */ export function assign_nodes(start, end) { - /** @type {Effect} */ (current_effect).nodes ??= { start, end }; + if (/** @type {Effect} */ (current_effect).n_start === null) { + /** @type {Effect} */ (current_effect).n_start = start; + /** @type {Effect} */ (current_effect).n_end = end; + } } /** @@ -255,7 +258,7 @@ export function comment() { */ export function append(anchor, dom) { if (hydrating) { - /** @type {Effect & { nodes: EffectNodes }} */ (current_effect).nodes.end = hydrate_node; + /** @type {Effect} */ (current_effect).n_end = hydrate_node; hydrate_next(); return; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 68a309cbdacd..6378c3408856 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -95,7 +95,8 @@ function create_effect(type, fn, sync, push = true) { var effect = { ctx: current_component_context, deps: null, - nodes: null, + n_start: null, + n_end: null, f: type | DIRTY, first: null, fn, @@ -135,7 +136,7 @@ function create_effect(type, fn, sync, push = true) { sync && effect.deps === null && effect.first === null && - effect.nodes === null && + effect.n_start === null && effect.teardown === null; if (!inert && !is_root && push) { @@ -355,10 +356,10 @@ export function execute_effect_teardown(effect) { export function destroy_effect(effect, remove_dom = true) { var removed = false; - if ((remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.nodes !== null) { + if ((remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.n_start !== null) { /** @type {TemplateNode | null} */ - var node = effect.nodes.start; - var end = effect.nodes.end; + var node = effect.n_start; + var end = effect.n_end; while (node !== null) { /** @type {TemplateNode | null} */ @@ -398,7 +399,8 @@ export function destroy_effect(effect, remove_dom = true) { effect.deps = effect.parent = effect.fn = - effect.nodes = + effect.n_start = + effect.n_end = null; } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 4a53a7004318..30dfc5d0cd1b 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -34,11 +34,6 @@ export interface Derived extends Value, Reaction { deriveds: null | Derived[]; } -export interface EffectNodes { - start: TemplateNode; - end: null | TemplateNode; -} - export interface Effect extends Reaction { parent: Effect | null; /** @@ -47,7 +42,8 @@ export interface Effect extends Reaction { * block is reconciled. In the case of a single text/element node, * `start` and `end` will be the same. */ - nodes: null | EffectNodes; + n_start: null | TemplateNode; + n_end: null | TemplateNode; /** The associated component context */ ctx: null | ComponentContext; /** The effect function */ diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 044c09e3105b..b6c6e9a9260f 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1,4 +1,4 @@ -/** @import { ComponentContext, Effect, EffectNodes, TemplateNode } from '#client' */ +/** @import { ComponentContext, Effect, TemplateNode } from '#client' */ /** @import { Component, ComponentType, SvelteComponent } from '../../index.js' */ import { DEV } from 'esm-env'; import { @@ -246,7 +246,7 @@ function _mount(Component, { target, anchor, props = {}, events, context, intro should_intro = true; if (hydrating) { - /** @type {Effect & { nodes: EffectNodes }} */ (current_effect).nodes.end = hydrate_node; + /** @type {Effect} */ (current_effect).n_end = hydrate_node; } if (context) { diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2ab833fc253a..41757ea45451 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -518,7 +518,7 @@ function flush_queued_effects(effects) { // don't know if we need to keep them until they are executed. Doing the check // here (rather than in `update_effect`) allows us to skip the work for // immediate effects. - if (effect.deps === null && effect.first === null && effect.nodes === null) { + if (effect.deps === null && effect.first === null && effect.n_start === null) { if (effect.teardown === null) { // remove this effect from the graph unlink_effect(effect); From 70f2d9314943cba3c5f182dc6edcc76fbc116a58 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 16 Aug 2024 16:48:53 +0100 Subject: [PATCH 2/7] Revert "chore: set `binding.kind` before analysis (#12843)" This reverts commit 19beb7754e82dd40052499d8c253e1fe2bb69cf8. --- .changeset/pink-countries-repair.md | 5 - packages/svelte/src/compiler/migrate/index.js | 7 +- .../src/compiler/phases/2-analyze/index.js | 115 +----------------- .../phases/2-analyze/visitors/Attribute.js | 4 +- .../2-analyze/visitors/BindDirective.js | 6 +- .../visitors/ExportNamedDeclaration.js | 31 +++++ .../2-analyze/visitors/ExportSpecifier.js | 24 +++- .../phases/2-analyze/visitors/Identifier.js | 41 ++++++- .../2-analyze/visitors/StyleDirective.js | 4 + .../phases/3-transform/client/utils.js | 2 +- .../3-transform/client/visitors/EachBlock.js | 6 +- packages/svelte/src/compiler/phases/scope.js | 42 +++++-- packages/svelte/src/compiler/types/index.d.ts | 2 - .../svelte/src/compiler/types/template.d.ts | 2 + packages/svelte/types/index.d.ts | 4 +- 15 files changed, 152 insertions(+), 143 deletions(-) delete mode 100644 .changeset/pink-countries-repair.md diff --git a/.changeset/pink-countries-repair.md b/.changeset/pink-countries-repair.md deleted file mode 100644 index faef646ba196..000000000000 --- a/.changeset/pink-countries-repair.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -chore: set `binding.kind` before analysis diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 31aee08cfa8c..cc5344e5c23a 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -331,7 +331,10 @@ const instance_script = { const binding = /** @type {Compiler.Binding} */ (state.scope.get(declarator.id.name)); - if (state.analysis.uses_props && (declarator.init || binding.updated)) { + if ( + state.analysis.uses_props && + (declarator.init || binding.mutated || binding.reassigned) + ) { throw new Error( '$$props is used together with named props in a way that cannot be automatically migrated.' ); @@ -347,7 +350,7 @@ const instance_script = { ) : '', optional: !!declarator.init, - bindable: binding.updated, + bindable: binding.mutated || binding.reassigned, ...extract_type_and_comment(declarator, state.str, path) }); state.props_insertion_point = /** @type {number} */ (declarator.end); diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 9429d3f6d265..0eb6c0fa9324 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1,13 +1,13 @@ -/** @import { Expression, Node, Program } from 'estree' */ -/** @import { Binding, Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */ +/** @import { Node, Program } from 'estree' */ +/** @import { Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */ /** @import { AnalysisState, Visitors } from './types' */ /** @import { Analysis, ComponentAnalysis, Js, ReactiveStatement, Template } from '../types' */ import { walk } from 'zimmerframe'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; -import { extract_identifiers, is_text_attribute } from '../../utils/ast.js'; +import { is_text_attribute } from '../../utils/ast.js'; import * as b from '../../utils/builders.js'; -import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js'; +import { Scope, ScopeRoot, create_scopes, get_rune } from '../scope.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; import { create_attribute } from '../nodes.js'; import { analyze_css } from './css/css-analyze.js'; @@ -63,7 +63,6 @@ import { TitleElement } from './visitors/TitleElement.js'; import { UpdateExpression } from './visitors/UpdateExpression.js'; import { UseDirective } from './visitors/UseDirective.js'; import { VariableDeclarator } from './visitors/VariableDeclarator.js'; -import is_reference from 'is-reference'; /** * @type {Visitors} @@ -400,112 +399,6 @@ export function analyze_component(root, source, options) { source }; - if (!runes) { - // every exported `let` or `var` declaration becomes a prop, everything else becomes an export - for (const node of instance.ast.body) { - if (node.type !== 'ExportNamedDeclaration') continue; - - analysis.needs_props = true; - - if (node.declaration) { - if ( - node.declaration.type === 'FunctionDeclaration' || - node.declaration.type === 'ClassDeclaration' - ) { - analysis.exports.push({ - name: /** @type {import('estree').Identifier} */ (node.declaration.id).name, - alias: null - }); - } else if (node.declaration.type === 'VariableDeclaration') { - if (node.declaration.kind === 'const') { - for (const declarator of node.declaration.declarations) { - for (const node of extract_identifiers(declarator.id)) { - analysis.exports.push({ name: node.name, alias: null }); - } - } - } else { - for (const declarator of node.declaration.declarations) { - for (const id of extract_identifiers(declarator.id)) { - const binding = /** @type {Binding} */ (instance.scope.get(id.name)); - binding.kind = 'bindable_prop'; - } - } - } - } - } else { - for (const specifier of node.specifiers) { - const binding = instance.scope.get(specifier.local.name); - - if ( - binding && - (binding.declaration_kind === 'var' || binding.declaration_kind === 'let') - ) { - binding.kind = 'bindable_prop'; - - if (specifier.exported.name !== specifier.local.name) { - binding.prop_alias = specifier.exported.name; - } - } else { - analysis.exports.push({ name: specifier.local.name, alias: specifier.exported.name }); - } - } - } - } - - // if reassigned/mutated bindings are referenced in `$:` blocks - // or the template, turn them into state - for (const binding of instance.scope.declarations.values()) { - if (binding.kind !== 'normal') continue; - - for (const { node, path } of binding.references) { - if (node === binding.node) continue; - - if (binding.updated) { - if ( - path[path.length - 1].type === 'StyleDirective' || - path.some((node) => node.type === 'Fragment') || - (path[1].type === 'LabeledStatement' && path[1].label.name === '$') - ) { - binding.kind = 'state'; - } - } - } - } - - // more legacy nonsense: if an `each` binding is reassigned/mutated, - // treat the expression as being mutated as well - walk(/** @type {SvelteNode} */ (template.ast), null, { - EachBlock(node) { - const scope = /** @type {Scope} */ (template.scopes.get(node)); - - for (const binding of scope.declarations.values()) { - if (binding.updated) { - const state = { scope: /** @type {Scope} */ (scope.parent), scopes: template.scopes }; - - walk(node.expression, state, { - // @ts-expect-error - _: set_scope, - Identifier(node, context) { - const parent = /** @type {Expression} */ (context.path.at(-1)); - - if (is_reference(node, parent)) { - const binding = context.state.scope.get(node.name); - - if (binding && binding.kind === 'normal') { - binding.kind = 'state'; - binding.mutated = binding.updated = true; - } - } - } - }); - - break; - } - } - } - }); - } - if (root.options) { for (const attribute of root.options.attributes) { if (attribute.name === 'accessors') { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index 9bcb266eaf62..8293f136d826 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -143,7 +143,7 @@ function get_delegated_event(event_name, handler, context) { return unhoisted; } - if (binding !== null && binding.initial !== null && !binding.updated && !binding.is_called) { + if (binding !== null && binding.initial !== null && !binding.mutated && !binding.is_called) { const binding_type = binding.initial.type; if ( @@ -197,7 +197,7 @@ function get_delegated_event(event_name, handler, context) { (((!context.state.analysis.runes && binding.kind === 'each') || // or any normal not reactive bindings that are mutated. binding.kind === 'normal') && - binding.updated)) + binding.mutated)) ) { return unhoisted; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js index 43d1a3d46844..c2becffecc53 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -39,7 +39,7 @@ export function BindDirective(node, context) { binding.kind !== 'bindable_prop' && binding.kind !== 'each' && binding.kind !== 'store_sub' && - !binding.updated)) // TODO wut? + !binding.mutated)) ) { e.bind_invalid_value(node.expression); } @@ -80,6 +80,10 @@ export function BindDirective(node, context) { if (references.length > 0) { parent.metadata.contains_group_binding = true; + for (const binding of parent.metadata.references) { + binding.mutated = true; + } + each_blocks.push(parent); ids = ids.filter((id) => !references.includes(id)); ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js index 547f6ab9c73e..e50de112e093 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js @@ -39,6 +39,37 @@ export function ExportNamedDeclaration(node, context) { } } + if (context.state.ast_type === 'instance' && !context.state.analysis.runes) { + context.state.analysis.needs_props = true; + + if (node.declaration) { + if ( + node.declaration.type === 'FunctionDeclaration' || + node.declaration.type === 'ClassDeclaration' + ) { + context.state.analysis.exports.push({ + name: /** @type {Identifier} */ (node.declaration.id).name, + alias: null + }); + } else if (node.declaration.type === 'VariableDeclaration') { + if (node.declaration.kind === 'const') { + for (const declarator of node.declaration.declarations) { + for (const node of extract_identifiers(declarator.id)) { + context.state.analysis.exports.push({ name: node.name, alias: null }); + } + } + } else { + for (const declarator of node.declaration.declarations) { + for (const id of extract_identifiers(declarator.id)) { + const binding = /** @type {Binding} */ (context.state.scope.get(id.name)); + binding.kind = 'bindable_prop'; + } + } + } + } + } + } + if (context.state.analysis.runes) { if (node.declaration && context.state.ast_type === 'instance') { if ( diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js index 27eb39acbfe2..320b81d0863d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js @@ -17,7 +17,29 @@ export function ExportSpecifier(node, context) { }); const binding = context.state.scope.get(node.local.name); - if (binding) binding.reassigned = binding.updated = true; + if (binding) binding.reassigned = true; + } else { + context.state.analysis.needs_props = true; + + const binding = /** @type {Binding} */ (context.state.scope.get(node.local.name)); + + if ( + binding !== null && + (binding.kind === 'state' || + binding.kind === 'raw_state' || + (binding.kind === 'normal' && + (binding.declaration_kind === 'let' || binding.declaration_kind === 'var'))) + ) { + binding.kind = 'bindable_prop'; + if (node.exported.name !== node.local.name) { + binding.prop_alias = node.exported.name; + } + } else { + context.state.analysis.exports.push({ + name: node.local.name, + alias: node.exported.name + }); + } } } else { validate_export(node, context.state.scope, node.local.name); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index 1f3406e92e28..9196ffc5f925 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -1,5 +1,4 @@ /** @import { Expression, Identifier } from 'estree' */ -/** @import { EachBlock } from '#compiler' */ /** @import { Context } from '../types' */ import is_reference from 'is-reference'; import { should_proxy } from '../../3-transform/client/utils.js'; @@ -81,6 +80,46 @@ export function Identifier(node, context) { if (node.name === '$$restProps') { context.state.analysis.uses_rest_props = true; } + + if ( + binding?.kind === 'normal' && + ((binding.scope === context.state.instance_scope && + binding.declaration_kind !== 'function') || + binding.declaration_kind === 'import') + ) { + if ( + binding.declaration_kind !== 'import' && + binding.mutated && + // TODO could be more fine-grained - not every mention in the template implies a state binding + (context.state.reactive_statement || context.state.ast_type === 'template') + ) { + binding.kind = 'state'; + } else if ( + context.state.reactive_statement && + parent.type === 'AssignmentExpression' && + parent.left === binding.node + ) { + binding.kind = 'derived'; + } + } else if (binding?.kind === 'each' && binding.mutated) { + // Ensure that the array is marked as reactive even when only its entries are mutated + let i = context.path.length; + while (i--) { + const ancestor = context.path[i]; + if ( + ancestor.type === 'EachBlock' && + context.state.analysis.template.scopes.get(ancestor)?.declarations.get(node.name) === + binding + ) { + for (const binding of ancestor.metadata.references) { + if (binding.kind === 'normal') { + binding.kind = 'state'; + } + } + break; + } + } + } } if (binding) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js index d79722f25698..f69ff67aa89d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js @@ -18,6 +18,10 @@ export function StyleDirective(node, context) { let binding = context.state.scope.get(node.name); if (binding) { + if (!context.state.analysis.runes && binding.mutated) { + binding.kind = 'state'; + } + if (binding.kind !== 'normal') { node.metadata.expression.has_state = true; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 35283aed72d3..5d2d60462591 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -236,7 +236,7 @@ export function is_prop_source(binding, state) { binding.initial || // Until legacy mode is gone, we also need to use the prop source when only mutated is true, // because the parent could be a legacy component which needs coarse-grained reactivity - binding.updated) + binding.mutated) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index d7876651217a..ae7528986697 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -114,7 +114,7 @@ export function EachBlock(node, context) { const indirect_dependencies = collect_parent_each_blocks(context).flatMap((block) => { const array = /** @type {Expression} */ (context.visit(block.expression)); const transitive_dependencies = build_transitive_dependencies( - block.metadata.expression.dependencies, + block.metadata.references, context ); return [array, ...transitive_dependencies]; @@ -126,7 +126,7 @@ export function EachBlock(node, context) { indirect_dependencies.push(collection); const transitive_dependencies = build_transitive_dependencies( - each_node_meta.expression.dependencies, + each_node_meta.references, context ); indirect_dependencies.push(...transitive_dependencies); @@ -293,7 +293,7 @@ function collect_parent_each_blocks(context) { } /** - * @param {Set} references + * @param {Binding[]} references * @param {ComponentContext} context */ function build_transitive_dependencies(references, context) { diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 66a49f87c7fd..3711474e5a2f 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -113,14 +113,13 @@ export class Scope { references: [], legacy_dependencies: [], initial, - reassigned: false, mutated: false, - updated: false, scope: this, kind, declaration_kind, is_called: false, prop_alias: null, + reassigned: false, metadata: null }; this.declarations.set(node.name, binding); @@ -507,7 +506,17 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { }, EachBlock(node, { state, visit }) { + // Array part is still from the scope above + /** @type {Set} */ + const references_within = new Set(); + const idx = references.length; visit(node.expression); + for (let i = idx; i < references.length; i++) { + const [scope, { node: id }] = references[i]; + if (scope === state.scope) { + references_within.add(id); + } + } // context and children are a new scope const scope = state.scope.child(); @@ -573,6 +582,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { index: scope.root.unique('$$index'), item: node.context.type === 'Identifier' ? node.context : b.id('$$item'), declarations: scope.declarations, + references: [...references_within] + .map((id) => /** @type {Binding} */ (state.scope.get(id.name))) + .filter(Boolean), is_controlled: false }; }, @@ -659,7 +671,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { // the css property StyleDirective(node, { path, state, next }) { if (node.value === true) { - state.scope.reference(b.id(node.name), path.concat(node)); + state.scope.reference(b.id(node.name), path); } next(); } @@ -681,19 +693,25 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { } for (const [scope, node] of updates) { - for (const expression of unwrap_pattern(node)) { - const left = object(expression); - const binding = left && scope.get(left.name); + if (node.type === 'MemberExpression') { + let object = node.object; + while (object.type === 'MemberExpression') { + object = object.object; + } - if (binding !== null && left !== binding.node) { - binding.updated = true; + const binding = scope.get(/** @type {Identifier} */ (object).name); + if (binding) binding.mutated = true; + } else { + unwrap_pattern(node).forEach((node) => { + let id = node.type === 'Identifier' ? node : object(node); + if (id === null) return; - if (left === expression) { - binding.reassigned = true; - } else { + const binding = scope.get(id.name); + if (binding && id !== binding.node) { binding.mutated = true; + binding.reassigned = node.type === 'Identifier'; } - } + }); } } diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 587c9e373cc7..5406dc5fe152 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -300,8 +300,6 @@ export interface Binding { references: { node: Identifier; path: SvelteNode[] }[]; mutated: boolean; reassigned: boolean; - /** `true` if mutated _or_ reassigned */ - updated: boolean; scope: Scope; /** For `legacy_reactive`: its reactive dependencies */ legacy_dependencies: Binding[]; diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index c803dfbba96a..58938f2474b4 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -407,6 +407,8 @@ export interface EachBlock extends BaseNode { index: Identifier; item: Identifier; declarations: Map; + /** List of bindings that are referenced within the expression */ + references: Binding[]; /** * Optimization path for each blocks: If the parent isn't a fragment and * it only has a single child, then we can classify the block as being "controlled". diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index b7412c46e573..43f5486781de 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -949,8 +949,6 @@ declare module 'svelte/compiler' { references: { node: Identifier; path: SvelteNode[] }[]; mutated: boolean; reassigned: boolean; - /** `true` if mutated _or_ reassigned */ - updated: boolean; scope: Scope; /** For `legacy_reactive`: its reactive dependencies */ legacy_dependencies: Binding[]; @@ -1864,6 +1862,8 @@ declare module 'svelte/compiler' { index: Identifier; item: Identifier; declarations: Map; + /** List of bindings that are referenced within the expression */ + references: Binding[]; /** * Optimization path for each blocks: If the parent isn't a fragment and * it only has a single child, then we can classify the block as being "controlled". From 07899e4c143c11ac01b2b924e3340eabd5a6ecfc Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 17 Aug 2024 12:45:05 +0100 Subject: [PATCH 3/7] name better --- .../svelte/src/internal/client/dom/blocks/each.js | 8 ++++---- .../src/internal/client/dom/blocks/svelte-element.js | 2 +- packages/svelte/src/internal/client/dom/template.js | 8 ++++---- .../svelte/src/internal/client/reactivity/effects.js | 12 ++++++------ .../svelte/src/internal/client/reactivity/types.d.ts | 4 ++-- packages/svelte/src/internal/client/render.js | 2 +- packages/svelte/src/internal/client/runtime.js | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 85500de7e0d4..0370a6404713 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -288,7 +288,7 @@ function reconcile(array, state, anchor, render_fn, flags, get_key) { item = items.get(key); if (item === undefined) { - var child_anchor = current ? /** @type {TemplateNode} */ (current.e.n_start) : anchor; + var child_anchor = current ? /** @type {TemplateNode} */ (current.e.nodes_start) : anchor; prev = create_item( child_anchor, @@ -509,10 +509,10 @@ function create_item(anchor, state, prev, next, value, key, index, render_fn, fl * @param {Text | Element | Comment} anchor */ function move(item, next, anchor) { - var end = item.next ? /** @type {TemplateNode} */ (item.next.e.n_start) : anchor; + var end = item.next ? /** @type {TemplateNode} */ (item.next.e.nodes_start) : anchor; - var dest = next ? /** @type {TemplateNode} */ (next.e.n_start) : anchor; - var node = /** @type {TemplateNode} */ (item.e.n_start); + var dest = next ? /** @type {TemplateNode} */ (next.e.nodes_start) : anchor; + var node = /** @type {TemplateNode} */ (item.e.nodes_start); while (node !== end) { var next_node = /** @type {TemplateNode} */ (get_next_sibling(node)); diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index c602b274d550..f8edafa0878c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -138,7 +138,7 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio } // we do this after calling `render_fn` so that child effects don't override `nodes.end` - /** @type {Effect} */ (current_effect).n_end = element; + /** @type {Effect} */ (current_effect).nodes_end = element; anchor.before(element); }); diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 8bd0dbb72544..0ff89360ea0b 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -11,9 +11,9 @@ import { queue_micro_task } from './task.js'; * @param {TemplateNode | null} end */ export function assign_nodes(start, end) { - if (/** @type {Effect} */ (current_effect).n_start === null) { - /** @type {Effect} */ (current_effect).n_start = start; - /** @type {Effect} */ (current_effect).n_end = end; + if (/** @type {Effect} */ (current_effect).nodes_start === null) { + /** @type {Effect} */ (current_effect).nodes_start = start; + /** @type {Effect} */ (current_effect).nodes_end = end; } } @@ -258,7 +258,7 @@ export function comment() { */ export function append(anchor, dom) { if (hydrating) { - /** @type {Effect} */ (current_effect).n_end = hydrate_node; + /** @type {Effect} */ (current_effect).nodes_end = hydrate_node; hydrate_next(); return; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 6378c3408856..a11afd30695a 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -136,7 +136,7 @@ function create_effect(type, fn, sync, push = true) { sync && effect.deps === null && effect.first === null && - effect.n_start === null && + effect.nodes_start === null && effect.teardown === null; if (!inert && !is_root && push) { @@ -356,10 +356,10 @@ export function execute_effect_teardown(effect) { export function destroy_effect(effect, remove_dom = true) { var removed = false; - if ((remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.n_start !== null) { + if ((remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.nodes_start !== null) { /** @type {TemplateNode | null} */ - var node = effect.n_start; - var end = effect.n_end; + var node = effect.nodes_start; + var end = effect.nodes_end; while (node !== null) { /** @type {TemplateNode | null} */ @@ -399,8 +399,8 @@ export function destroy_effect(effect, remove_dom = true) { effect.deps = effect.parent = effect.fn = - effect.n_start = - effect.n_end = + effect.nodes_start = + effect.nodes_end = null; } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 30dfc5d0cd1b..d95126d4d3f4 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -42,8 +42,8 @@ export interface Effect extends Reaction { * block is reconciled. In the case of a single text/element node, * `start` and `end` will be the same. */ - n_start: null | TemplateNode; - n_end: null | TemplateNode; + nodes_start: null | TemplateNode; + nodes_end: null | TemplateNode; /** The associated component context */ ctx: null | ComponentContext; /** The effect function */ diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index b6c6e9a9260f..4afd10a7dcdc 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -246,7 +246,7 @@ function _mount(Component, { target, anchor, props = {}, events, context, intro should_intro = true; if (hydrating) { - /** @type {Effect} */ (current_effect).n_end = hydrate_node; + /** @type {Effect} */ (current_effect).nodes_end = hydrate_node; } if (context) { diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 41757ea45451..75896ccc078d 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -518,7 +518,7 @@ function flush_queued_effects(effects) { // don't know if we need to keep them until they are executed. Doing the check // here (rather than in `update_effect`) allows us to skip the work for // immediate effects. - if (effect.deps === null && effect.first === null && effect.n_start === null) { + if (effect.deps === null && effect.first === null && effect.nodes_start === null) { if (effect.teardown === null) { // remove this effect from the graph unlink_effect(effect); From b41c5bf55914463dc1d98fdce69f8fd3d54009b4 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 17 Aug 2024 12:48:49 +0100 Subject: [PATCH 4/7] oops --- packages/svelte/src/internal/client/dom/template.js | 7 ++++--- packages/svelte/src/internal/client/reactivity/effects.js | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 0ff89360ea0b..796b8da1a5b7 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -11,9 +11,10 @@ import { queue_micro_task } from './task.js'; * @param {TemplateNode | null} end */ export function assign_nodes(start, end) { - if (/** @type {Effect} */ (current_effect).nodes_start === null) { - /** @type {Effect} */ (current_effect).nodes_start = start; - /** @type {Effect} */ (current_effect).nodes_end = end; + var effect = /** @type {Effect} */ (current_effect); + if (effect.nodes_start === null) { + effect.nodes_start = start; + effect.nodes_end = end; } } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index a11afd30695a..b274dbeed90e 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -95,8 +95,8 @@ function create_effect(type, fn, sync, push = true) { var effect = { ctx: current_component_context, deps: null, - n_start: null, - n_end: null, + nodes_start: null, + nodes_end: null, f: type | DIRTY, first: null, fn, From 76f66f83fe97a7d1c996c87b5eaa554558327cc7 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 17 Aug 2024 14:31:53 +0100 Subject: [PATCH 5/7] revert --- packages/svelte/src/compiler/migrate/index.js | 7 +- .../src/compiler/phases/2-analyze/index.js | 115 +++++++++++++++++- .../phases/2-analyze/visitors/Attribute.js | 4 +- .../2-analyze/visitors/BindDirective.js | 6 +- .../visitors/ExportNamedDeclaration.js | 31 ----- .../2-analyze/visitors/ExportSpecifier.js | 24 +--- .../phases/2-analyze/visitors/Identifier.js | 41 +------ .../2-analyze/visitors/StyleDirective.js | 4 - .../phases/3-transform/client/utils.js | 2 +- .../3-transform/client/visitors/EachBlock.js | 6 +- packages/svelte/src/compiler/phases/scope.js | 42 ++----- packages/svelte/src/compiler/types/index.d.ts | 2 + .../svelte/src/compiler/types/template.d.ts | 2 - 13 files changed, 136 insertions(+), 150 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index cc5344e5c23a..31aee08cfa8c 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -331,10 +331,7 @@ const instance_script = { const binding = /** @type {Compiler.Binding} */ (state.scope.get(declarator.id.name)); - if ( - state.analysis.uses_props && - (declarator.init || binding.mutated || binding.reassigned) - ) { + if (state.analysis.uses_props && (declarator.init || binding.updated)) { throw new Error( '$$props is used together with named props in a way that cannot be automatically migrated.' ); @@ -350,7 +347,7 @@ const instance_script = { ) : '', optional: !!declarator.init, - bindable: binding.mutated || binding.reassigned, + bindable: binding.updated, ...extract_type_and_comment(declarator, state.str, path) }); state.props_insertion_point = /** @type {number} */ (declarator.end); diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 0eb6c0fa9324..9429d3f6d265 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1,13 +1,13 @@ -/** @import { Node, Program } from 'estree' */ -/** @import { Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */ +/** @import { Expression, Node, Program } from 'estree' */ +/** @import { Binding, Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */ /** @import { AnalysisState, Visitors } from './types' */ /** @import { Analysis, ComponentAnalysis, Js, ReactiveStatement, Template } from '../types' */ import { walk } from 'zimmerframe'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; -import { is_text_attribute } from '../../utils/ast.js'; +import { extract_identifiers, is_text_attribute } from '../../utils/ast.js'; import * as b from '../../utils/builders.js'; -import { Scope, ScopeRoot, create_scopes, get_rune } from '../scope.js'; +import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; import { create_attribute } from '../nodes.js'; import { analyze_css } from './css/css-analyze.js'; @@ -63,6 +63,7 @@ import { TitleElement } from './visitors/TitleElement.js'; import { UpdateExpression } from './visitors/UpdateExpression.js'; import { UseDirective } from './visitors/UseDirective.js'; import { VariableDeclarator } from './visitors/VariableDeclarator.js'; +import is_reference from 'is-reference'; /** * @type {Visitors} @@ -399,6 +400,112 @@ export function analyze_component(root, source, options) { source }; + if (!runes) { + // every exported `let` or `var` declaration becomes a prop, everything else becomes an export + for (const node of instance.ast.body) { + if (node.type !== 'ExportNamedDeclaration') continue; + + analysis.needs_props = true; + + if (node.declaration) { + if ( + node.declaration.type === 'FunctionDeclaration' || + node.declaration.type === 'ClassDeclaration' + ) { + analysis.exports.push({ + name: /** @type {import('estree').Identifier} */ (node.declaration.id).name, + alias: null + }); + } else if (node.declaration.type === 'VariableDeclaration') { + if (node.declaration.kind === 'const') { + for (const declarator of node.declaration.declarations) { + for (const node of extract_identifiers(declarator.id)) { + analysis.exports.push({ name: node.name, alias: null }); + } + } + } else { + for (const declarator of node.declaration.declarations) { + for (const id of extract_identifiers(declarator.id)) { + const binding = /** @type {Binding} */ (instance.scope.get(id.name)); + binding.kind = 'bindable_prop'; + } + } + } + } + } else { + for (const specifier of node.specifiers) { + const binding = instance.scope.get(specifier.local.name); + + if ( + binding && + (binding.declaration_kind === 'var' || binding.declaration_kind === 'let') + ) { + binding.kind = 'bindable_prop'; + + if (specifier.exported.name !== specifier.local.name) { + binding.prop_alias = specifier.exported.name; + } + } else { + analysis.exports.push({ name: specifier.local.name, alias: specifier.exported.name }); + } + } + } + } + + // if reassigned/mutated bindings are referenced in `$:` blocks + // or the template, turn them into state + for (const binding of instance.scope.declarations.values()) { + if (binding.kind !== 'normal') continue; + + for (const { node, path } of binding.references) { + if (node === binding.node) continue; + + if (binding.updated) { + if ( + path[path.length - 1].type === 'StyleDirective' || + path.some((node) => node.type === 'Fragment') || + (path[1].type === 'LabeledStatement' && path[1].label.name === '$') + ) { + binding.kind = 'state'; + } + } + } + } + + // more legacy nonsense: if an `each` binding is reassigned/mutated, + // treat the expression as being mutated as well + walk(/** @type {SvelteNode} */ (template.ast), null, { + EachBlock(node) { + const scope = /** @type {Scope} */ (template.scopes.get(node)); + + for (const binding of scope.declarations.values()) { + if (binding.updated) { + const state = { scope: /** @type {Scope} */ (scope.parent), scopes: template.scopes }; + + walk(node.expression, state, { + // @ts-expect-error + _: set_scope, + Identifier(node, context) { + const parent = /** @type {Expression} */ (context.path.at(-1)); + + if (is_reference(node, parent)) { + const binding = context.state.scope.get(node.name); + + if (binding && binding.kind === 'normal') { + binding.kind = 'state'; + binding.mutated = binding.updated = true; + } + } + } + }); + + break; + } + } + } + }); + } + if (root.options) { for (const attribute of root.options.attributes) { if (attribute.name === 'accessors') { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index 8293f136d826..9bcb266eaf62 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -143,7 +143,7 @@ function get_delegated_event(event_name, handler, context) { return unhoisted; } - if (binding !== null && binding.initial !== null && !binding.mutated && !binding.is_called) { + if (binding !== null && binding.initial !== null && !binding.updated && !binding.is_called) { const binding_type = binding.initial.type; if ( @@ -197,7 +197,7 @@ function get_delegated_event(event_name, handler, context) { (((!context.state.analysis.runes && binding.kind === 'each') || // or any normal not reactive bindings that are mutated. binding.kind === 'normal') && - binding.mutated)) + binding.updated)) ) { return unhoisted; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js index c2becffecc53..43d1a3d46844 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -39,7 +39,7 @@ export function BindDirective(node, context) { binding.kind !== 'bindable_prop' && binding.kind !== 'each' && binding.kind !== 'store_sub' && - !binding.mutated)) + !binding.updated)) // TODO wut? ) { e.bind_invalid_value(node.expression); } @@ -80,10 +80,6 @@ export function BindDirective(node, context) { if (references.length > 0) { parent.metadata.contains_group_binding = true; - for (const binding of parent.metadata.references) { - binding.mutated = true; - } - each_blocks.push(parent); ids = ids.filter((id) => !references.includes(id)); ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js index e50de112e093..547f6ab9c73e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportNamedDeclaration.js @@ -39,37 +39,6 @@ export function ExportNamedDeclaration(node, context) { } } - if (context.state.ast_type === 'instance' && !context.state.analysis.runes) { - context.state.analysis.needs_props = true; - - if (node.declaration) { - if ( - node.declaration.type === 'FunctionDeclaration' || - node.declaration.type === 'ClassDeclaration' - ) { - context.state.analysis.exports.push({ - name: /** @type {Identifier} */ (node.declaration.id).name, - alias: null - }); - } else if (node.declaration.type === 'VariableDeclaration') { - if (node.declaration.kind === 'const') { - for (const declarator of node.declaration.declarations) { - for (const node of extract_identifiers(declarator.id)) { - context.state.analysis.exports.push({ name: node.name, alias: null }); - } - } - } else { - for (const declarator of node.declaration.declarations) { - for (const id of extract_identifiers(declarator.id)) { - const binding = /** @type {Binding} */ (context.state.scope.get(id.name)); - binding.kind = 'bindable_prop'; - } - } - } - } - } - } - if (context.state.analysis.runes) { if (node.declaration && context.state.ast_type === 'instance') { if ( diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js index 320b81d0863d..27eb39acbfe2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExportSpecifier.js @@ -17,29 +17,7 @@ export function ExportSpecifier(node, context) { }); const binding = context.state.scope.get(node.local.name); - if (binding) binding.reassigned = true; - } else { - context.state.analysis.needs_props = true; - - const binding = /** @type {Binding} */ (context.state.scope.get(node.local.name)); - - if ( - binding !== null && - (binding.kind === 'state' || - binding.kind === 'raw_state' || - (binding.kind === 'normal' && - (binding.declaration_kind === 'let' || binding.declaration_kind === 'var'))) - ) { - binding.kind = 'bindable_prop'; - if (node.exported.name !== node.local.name) { - binding.prop_alias = node.exported.name; - } - } else { - context.state.analysis.exports.push({ - name: node.local.name, - alias: node.exported.name - }); - } + if (binding) binding.reassigned = binding.updated = true; } } else { validate_export(node, context.state.scope, node.local.name); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index 9196ffc5f925..1f3406e92e28 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -1,4 +1,5 @@ /** @import { Expression, Identifier } from 'estree' */ +/** @import { EachBlock } from '#compiler' */ /** @import { Context } from '../types' */ import is_reference from 'is-reference'; import { should_proxy } from '../../3-transform/client/utils.js'; @@ -80,46 +81,6 @@ export function Identifier(node, context) { if (node.name === '$$restProps') { context.state.analysis.uses_rest_props = true; } - - if ( - binding?.kind === 'normal' && - ((binding.scope === context.state.instance_scope && - binding.declaration_kind !== 'function') || - binding.declaration_kind === 'import') - ) { - if ( - binding.declaration_kind !== 'import' && - binding.mutated && - // TODO could be more fine-grained - not every mention in the template implies a state binding - (context.state.reactive_statement || context.state.ast_type === 'template') - ) { - binding.kind = 'state'; - } else if ( - context.state.reactive_statement && - parent.type === 'AssignmentExpression' && - parent.left === binding.node - ) { - binding.kind = 'derived'; - } - } else if (binding?.kind === 'each' && binding.mutated) { - // Ensure that the array is marked as reactive even when only its entries are mutated - let i = context.path.length; - while (i--) { - const ancestor = context.path[i]; - if ( - ancestor.type === 'EachBlock' && - context.state.analysis.template.scopes.get(ancestor)?.declarations.get(node.name) === - binding - ) { - for (const binding of ancestor.metadata.references) { - if (binding.kind === 'normal') { - binding.kind = 'state'; - } - } - break; - } - } - } } if (binding) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js index f69ff67aa89d..d79722f25698 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js @@ -18,10 +18,6 @@ export function StyleDirective(node, context) { let binding = context.state.scope.get(node.name); if (binding) { - if (!context.state.analysis.runes && binding.mutated) { - binding.kind = 'state'; - } - if (binding.kind !== 'normal') { node.metadata.expression.has_state = true; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 5d2d60462591..35283aed72d3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -236,7 +236,7 @@ export function is_prop_source(binding, state) { binding.initial || // Until legacy mode is gone, we also need to use the prop source when only mutated is true, // because the parent could be a legacy component which needs coarse-grained reactivity - binding.mutated) + binding.updated) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index ae7528986697..d7876651217a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -114,7 +114,7 @@ export function EachBlock(node, context) { const indirect_dependencies = collect_parent_each_blocks(context).flatMap((block) => { const array = /** @type {Expression} */ (context.visit(block.expression)); const transitive_dependencies = build_transitive_dependencies( - block.metadata.references, + block.metadata.expression.dependencies, context ); return [array, ...transitive_dependencies]; @@ -126,7 +126,7 @@ export function EachBlock(node, context) { indirect_dependencies.push(collection); const transitive_dependencies = build_transitive_dependencies( - each_node_meta.references, + each_node_meta.expression.dependencies, context ); indirect_dependencies.push(...transitive_dependencies); @@ -293,7 +293,7 @@ function collect_parent_each_blocks(context) { } /** - * @param {Binding[]} references + * @param {Set} references * @param {ComponentContext} context */ function build_transitive_dependencies(references, context) { diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 3711474e5a2f..66a49f87c7fd 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -113,13 +113,14 @@ export class Scope { references: [], legacy_dependencies: [], initial, + reassigned: false, mutated: false, + updated: false, scope: this, kind, declaration_kind, is_called: false, prop_alias: null, - reassigned: false, metadata: null }; this.declarations.set(node.name, binding); @@ -506,17 +507,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { }, EachBlock(node, { state, visit }) { - // Array part is still from the scope above - /** @type {Set} */ - const references_within = new Set(); - const idx = references.length; visit(node.expression); - for (let i = idx; i < references.length; i++) { - const [scope, { node: id }] = references[i]; - if (scope === state.scope) { - references_within.add(id); - } - } // context and children are a new scope const scope = state.scope.child(); @@ -582,9 +573,6 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { index: scope.root.unique('$$index'), item: node.context.type === 'Identifier' ? node.context : b.id('$$item'), declarations: scope.declarations, - references: [...references_within] - .map((id) => /** @type {Binding} */ (state.scope.get(id.name))) - .filter(Boolean), is_controlled: false }; }, @@ -671,7 +659,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { // the css property StyleDirective(node, { path, state, next }) { if (node.value === true) { - state.scope.reference(b.id(node.name), path); + state.scope.reference(b.id(node.name), path.concat(node)); } next(); } @@ -693,25 +681,19 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { } for (const [scope, node] of updates) { - if (node.type === 'MemberExpression') { - let object = node.object; - while (object.type === 'MemberExpression') { - object = object.object; - } + for (const expression of unwrap_pattern(node)) { + const left = object(expression); + const binding = left && scope.get(left.name); - const binding = scope.get(/** @type {Identifier} */ (object).name); - if (binding) binding.mutated = true; - } else { - unwrap_pattern(node).forEach((node) => { - let id = node.type === 'Identifier' ? node : object(node); - if (id === null) return; + if (binding !== null && left !== binding.node) { + binding.updated = true; - const binding = scope.get(id.name); - if (binding && id !== binding.node) { + if (left === expression) { + binding.reassigned = true; + } else { binding.mutated = true; - binding.reassigned = node.type === 'Identifier'; } - }); + } } } diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 5406dc5fe152..587c9e373cc7 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -300,6 +300,8 @@ export interface Binding { references: { node: Identifier; path: SvelteNode[] }[]; mutated: boolean; reassigned: boolean; + /** `true` if mutated _or_ reassigned */ + updated: boolean; scope: Scope; /** For `legacy_reactive`: its reactive dependencies */ legacy_dependencies: Binding[]; diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 58938f2474b4..c803dfbba96a 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -407,8 +407,6 @@ export interface EachBlock extends BaseNode { index: Identifier; item: Identifier; declarations: Map; - /** List of bindings that are referenced within the expression */ - references: Binding[]; /** * Optimization path for each blocks: If the parent isn't a fragment and * it only has a single child, then we can classify the block as being "controlled". From d2f71361e5b95dfb40747b1ce6dcbb2409ce57de Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 17 Aug 2024 14:32:19 +0100 Subject: [PATCH 6/7] revert --- packages/svelte/types/index.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 43f5486781de..b7412c46e573 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -949,6 +949,8 @@ declare module 'svelte/compiler' { references: { node: Identifier; path: SvelteNode[] }[]; mutated: boolean; reassigned: boolean; + /** `true` if mutated _or_ reassigned */ + updated: boolean; scope: Scope; /** For `legacy_reactive`: its reactive dependencies */ legacy_dependencies: Binding[]; @@ -1862,8 +1864,6 @@ declare module 'svelte/compiler' { index: Identifier; item: Identifier; declarations: Map; - /** List of bindings that are referenced within the expression */ - references: Binding[]; /** * Optimization path for each blocks: If the parent isn't a fragment and * it only has a single child, then we can classify the block as being "controlled". From 4e2f60906cb70e86798f9f312d90ea336084918b Mon Sep 17 00:00:00 2001 From: Conduitry Date: Sat, 17 Aug 2024 10:07:28 -0400 Subject: [PATCH 7/7] revert --- .changeset/pink-countries-repair.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pink-countries-repair.md diff --git a/.changeset/pink-countries-repair.md b/.changeset/pink-countries-repair.md new file mode 100644 index 000000000000..faef646ba196 --- /dev/null +++ b/.changeset/pink-countries-repair.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: set `binding.kind` before analysis