diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 10753729b3..73bcdfc61d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -6,7 +6,12 @@ import { walk } from 'zimmerframe'; import { parse } from '../1-parse/acorn.js'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; -import { extract_identifiers, has_await_expression } from '../../utils/ast.js'; +import { + extract_identifiers, + has_await_expression, + object, + unwrap_pattern +} from '../../utils/ast.js'; import * as b from '#compiler/builders'; import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; @@ -689,6 +694,37 @@ export function analyze_component(root, source, options) { } if (instance.has_await) { + /** + * @param {ESTree.Expression} expression + * @param {Scope} scope + * @param {Set} touched + * @param {Set} seen + */ + const touch = (expression, scope, touched, seen = new Set()) => { + if (seen.has(expression)) return; + seen.add(expression); + + walk( + expression, + { scope }, + { + Identifier(node, context) { + const parent = /** @type {ESTree.Node} */ (context.path.at(-1)); + if (is_reference(node, parent)) { + const binding = context.state.scope.get(node.name); + if (binding) { + touched.add(binding); + + for (const assignment of binding.assignments) { + touch(assignment.value, assignment.scope, touched, seen); + } + } + } + } + } + ); + }; + /** * @param {ESTree.Node} node * @param {Set} seen @@ -699,6 +735,22 @@ export function analyze_component(root, source, options) { if (seen.has(node)) return; seen.add(node); + /** + * @param {ESTree.Pattern} node + * @param {Scope} scope + */ + function update(node, scope) { + for (const pattern of unwrap_pattern(node)) { + const node = object(pattern); + if (!node) return; + + const binding = scope.get(node.name); + if (!binding) return; + + writes.add(binding); + } + } + walk( node, { scope: instance.scope }, @@ -712,12 +764,34 @@ export function analyze_component(root, source, options) { } }, AssignmentExpression(node, context) { - // TODO mark writes + update(node.left, context.state.scope); + }, + UpdateExpression(node, context) { + update( + /** @type {ESTree.Identifier | ESTree.MemberExpression} */ (node.argument), + context.state.scope + ); }, CallExpression(node, context) { - // TODO deopt arguments, assume they are mutated - // TODO recurse into function definitions + // for now, assume everything touched by the callee ends up mutating the object + // TODO optimise this better + + // special case — no need to peek inside effects + const rune = get_rune(node, context.state.scope); + if (rune === '$effect') return; + + /** @type {Set} */ + const touched = new Set(); + touch(node, context.state.scope, touched); + + for (const b of touched) { + writes.add(b); + } }, + // don't look inside functions until they are called + ArrowFunctionExpression(_, context) {}, + FunctionDeclaration(_, context) {}, + FunctionExpression(_, context) {}, Identifier(node, context) { const parent = /** @type {ESTree.Node} */ (context.path.at(-1)); if (is_reference(node, parent)) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js index b5d94fb2ad..603c7bd1a8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js @@ -334,24 +334,14 @@ function transform_body(program, context) { for (const binding of s.declarations) { binding.blocker = blocker; } - } - - // TODO we likely need to account for updates that happen after the declaration, - // e.g. `let obj = $state()` followed by a later `obj = {...}`, otherwise - // a synchronous `{obj.foo}` will fail - for (const binding of context.state.scope.declarations.values()) { - // if the binding is updated (TODO or passed to a function, in which case it - // could be mutated), play it safe and block until the end. In future we - // could develop more sophisticated static analysis to optimise further - if (binding.updated) { - binding.blocker = b.member(promises, b.literal(statements.length - 1), true); + for (const binding of s.writes) { + // if a statement writes to a binding, any reads of that + // binding must wait for the statement + binding.blocker = blocker; } } } - // console.log('statements', statements); - // console.log('deriveds', deriveds); - return out; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Program.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Program.js index 3712783502..a442939287 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/Program.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/Program.js @@ -204,24 +204,14 @@ function transform_body(program, context) { for (const binding of s.declarations) { binding.blocker = blocker; } - } - - // TODO we likely need to account for updates that happen after the declaration, - // e.g. `let obj = $state()` followed by a later `obj = {...}`, otherwise - // a synchronous `{obj.foo}` will fail - for (const binding of context.state.scope.declarations.values()) { - // if the binding is updated (TODO or passed to a function, in which case it - // could be mutated), play it safe and block until the end. In future we - // could develop more sophisticated static analysis to optimise further - if (binding.updated) { - binding.blocker = b.member(promises, b.literal(statements.length - 1), true); + for (const binding of s.writes) { + // if a statement writes to a binding, any reads of that + // binding must wait for the statement + binding.blocker = blocker; } } } - // console.log('statements', statements); - // console.log('deriveds', deriveds); - return out; } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 9d99ea9ee6..c39803318d 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -108,6 +108,9 @@ export class Binding { /** @type {Array<{ node: Identifier; path: AST.SvelteNode[] }>} */ references = []; + /** @type {Array<{ value: Expression; scope: Scope }>} */ + assignments = []; + /** * For `legacy_reactive`: its reactive dependencies * @type {Binding[]} @@ -152,6 +155,10 @@ export class Binding { this.initial = initial; this.kind = kind; this.declaration_kind = declaration_kind; + + if (initial) { + this.assignments.push({ value: /** @type {Expression} */ (initial), scope }); + } } get updated() { @@ -868,7 +875,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { /** @type {[Scope, { node: Identifier; path: AST.SvelteNode[] }][]} */ const references = []; - /** @type {[Scope, Pattern | MemberExpression][]} */ + /** @type {[Scope, Pattern | MemberExpression, Expression][]} */ const updates = []; /** @@ -1056,12 +1063,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { // updates AssignmentExpression(node, { state, next }) { - updates.push([state.scope, node.left]); + updates.push([state.scope, node.left, node.right]); next(); }, UpdateExpression(node, { state, next }) { - updates.push([state.scope, /** @type {Identifier | MemberExpression} */ (node.argument)]); + const expression = /** @type {Identifier | MemberExpression} */ (node.argument); + updates.push([state.scope, expression, expression]); next(); }, @@ -1282,10 +1290,11 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { }, BindDirective(node, context) { - updates.push([ - context.state.scope, - /** @type {Identifier | MemberExpression} */ (node.expression) - ]); + if (node.expression.type !== 'SequenceExpression') { + const expression = /** @type {Identifier | MemberExpression} */ (node.expression); + updates.push([context.state.scope, expression, expression]); + } + context.next(); }, @@ -1320,7 +1329,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { scope.reference(node, path); } - for (const [scope, node] of updates) { + for (const [scope, node, value] of updates) { for (const expression of unwrap_pattern(node)) { const left = object(expression); const binding = left && scope.get(left.name); @@ -1328,6 +1337,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { if (binding !== null && left !== binding.node) { if (left === expression) { binding.reassigned = true; + binding.assignments.push({ value, scope }); } else { binding.mutated = true; } diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-module/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-module/_config.js index 5ab7a9aa8c..7ab79eb825 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-derived-module/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-module/_config.js @@ -67,15 +67,15 @@ export default test({ assert.deepEqual(logs, [ 'outside boundary 1', - 'template 42 1', '$effect.pre 42 1', '$effect 42 1', - 'template 84 2', + 'template 42 1', '$effect.pre 84 2', + 'template 84 2', 'outside boundary 2', '$effect 84 2', - 'template 86 2', '$effect.pre 86 2', + 'template 86 2', '$effect 86 2' ]); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived/_config.js index a439b082e9..c866dce406 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-derived/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-derived/_config.js @@ -33,15 +33,15 @@ export default test({ assert.deepEqual(logs, [ 'outside boundary 1', - 'template 1a 1', '$effect.pre 1a 1', '$effect 1a 1', - 'template 2a 2', + 'template 1a 1', '$effect.pre 2a 2', + 'template 2a 2', 'outside boundary 2', '$effect 2a 2', - 'template 2b 2', '$effect.pre 2b 2', + 'template 2b 2', '$effect 2b 2' ]); }