Skip to content

Commit 75fc09a

Browse files
Rich-Harristrueadm
andauthored
Proxied state (#9739)
* magic objects * read length eagerly — triggers reconciliation * nested magic * tests * more tests * fix array memory leak * skipped, partially passing array test * Fix each revert bad changes * more 1337 * eliminate closures * maybe this is unnecessary? * only create sources for own properties * more * rename stuff * shuffle things around * emit $.proxy * remove proxy helper from public API * only create sources for writable properties * update test * get tests passing * fix * remove state-not-mutated warning, which is no longer valid * track reassignments separately from mutations * update test - effects no longer fire on mutations unnecessarily * move util into utils file * move each logic into its own module; breathe sigh of relief * tweaks * more tweaks * improve runtime * fix mistake * ensure we proxy when assigning to state * fix test * handle frozen * create sources when reading proxy properties inside deriveds * only mutate in legacy mode * add immutable to transform state * remove unused second argument to derived * remove unused second argument to source, and runtime immutable option to createRoot (not sure what that was doing there?) * oops, backwards * dedicated binding.kind for legacy reactive imports * avoid using prop_source in more cases. bit hacky, could be tidier, but it works * distinguish between source and mutable_source * remove immutable option from mount * remove unused apparatus around immutable option * deprecate immutable * fix * tweak * better default value handling * remove unnecessary exports * whitespace is free * remove obsolete symbol from proxy * cleanup ts * improve runtime perf by removing version reads in has() * add missing proxy call * tune perf of has() more * ensure we only create sources in effect_active() * fix proxy of computed fields * simplify and fix issue with indexed each blocks * fix compiler errors around exported state * only create source for state that is reassigned * temporary fix, we should come up with something better than this * readonly props * fix test * add test for bind: * make readonly dev-only * docs * forbid setPrototypeOf * lol whoops --------- Co-authored-by: Rich Harris <[email protected]> Co-authored-by: Dominic Gannaway <[email protected]>
1 parent f0c47c3 commit 75fc09a

File tree

144 files changed

+2297
-1550
lines changed

Some content is hidden

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

144 files changed

+2297
-1550
lines changed

packages/svelte/src/compiler/errors.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,8 @@ const runes = {
167167
'invalid-legacy-export': () => `Cannot use \`export let\` in runes mode — use $props instead`,
168168
/** @param {string} rune */
169169
'invalid-rune-usage': (rune) => `Cannot use ${rune} rune in non-runes mode`,
170-
/** @param {string} rune */
171-
'invalid-rune-export': (rune) => `Cannot export value created with ${rune}`,
170+
'invalid-state-export': () => `Cannot export state if it is reassigned`,
171+
'invalid-derived-export': () => `Cannot export derived state`,
172172
'invalid-props-id': () => `$props() can only be used with an object destructuring pattern`,
173173
'invalid-props-pattern': () =>
174174
`$props() assignment must not contain nested properties or computed keys`,

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

+11-26
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ function get_delegated_event(node, context) {
183183
// or any normal not reactive bindings that are mutated.
184184
binding.kind === 'normal' ||
185185
// or any reactive imports (those are rewritten) (can only happen in legacy mode)
186-
(binding.kind === 'state' && binding.declaration_kind === 'import')) &&
186+
binding.kind === 'legacy_reactive_import') &&
187187
binding.mutated))
188188
) {
189189
return non_hoistable;
@@ -221,20 +221,13 @@ export function analyze_module(ast, options) {
221221
merge(set_scope(scopes), validation_runes_js, runes_scope_js_tweaker)
222222
);
223223

224-
// If we are in runes mode, then check for possible misuses of state runes
225-
for (const [, scope] of scopes) {
226-
for (const [name, binding] of scope.declarations) {
227-
if (binding.kind === 'state' && !binding.mutated) {
228-
warn(warnings, binding.node, [], 'state-not-mutated', name);
229-
}
230-
}
231-
}
232-
233224
return {
234225
module: { ast, scope, scopes },
235226
name: options.filename || 'module',
236227
warnings,
237-
accessors: false
228+
accessors: false,
229+
runes: true,
230+
immutable: true
238231
};
239232
}
240233

@@ -315,6 +308,10 @@ export function analyze_component(root, options) {
315308

316309
const component_name = get_component_name(options.filename ?? 'Component');
317310

311+
const runes =
312+
options.runes ??
313+
Array.from(module.scope.references).some(([name]) => Runes.includes(/** @type {any} */ (name)));
314+
318315
// TODO remove all the ?? stuff, we don't need it now that we're validating the config
319316
/** @type {import('../types.js').ComponentAnalysis} */
320317
const analysis = {
@@ -331,11 +328,8 @@ export function analyze_component(root, options) {
331328
component_name,
332329
get_css_hash: options.cssHash
333330
}),
334-
runes:
335-
options.runes ??
336-
Array.from(module.scope.references).some(([name]) =>
337-
Runes.includes(/** @type {any} */ (name))
338-
),
331+
runes,
332+
immutable: runes || options.immutable,
339333
exports: [],
340334
uses_props: false,
341335
uses_rest_props: false,
@@ -382,15 +376,6 @@ export function analyze_component(root, options) {
382376
merge(set_scope(scopes), validation_runes, runes_scope_tweaker, common_visitors)
383377
);
384378
}
385-
386-
// If we are in runes mode, then check for possible misuses of state runes
387-
for (const [, scope] of instance.scopes) {
388-
for (const [name, binding] of scope.declarations) {
389-
if (binding.kind === 'state' && !binding.mutated) {
390-
warn(warnings, binding.node, [], 'state-not-mutated', name);
391-
}
392-
}
393-
}
394379
} else {
395380
instance.scope.declare(b.id('$$props'), 'prop', 'synthetic');
396381
instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic');
@@ -553,7 +538,7 @@ const legacy_scope_tweaker = {
553538
(state.reactive_statement || state.ast_type === 'template') &&
554539
parent.type === 'MemberExpression'
555540
) {
556-
binding.kind = 'state';
541+
binding.kind = 'legacy_reactive_import';
557542
}
558543
} else if (
559544
binding.mutated &&

packages/svelte/src/compiler/phases/2-analyze/validation.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -649,8 +649,14 @@ export const validation_legacy = merge(validation, a11y_validators, {
649649
*/
650650
function validate_export(node, scope, name) {
651651
const binding = scope.get(name);
652-
if (binding && (binding.kind === 'derived' || binding.kind === 'state')) {
653-
error(node, 'invalid-rune-export', `$${binding.kind}`);
652+
if (!binding) return;
653+
654+
if (binding.kind === 'derived') {
655+
error(node, 'invalid-derived-export');
656+
}
657+
658+
if (binding.kind === 'state' && binding.reassigned) {
659+
error(node, 'invalid-state-export');
654660
}
655661
}
656662

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

+7-13
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ export function client_component(source, analysis, options) {
159159
// Very very dirty way of making import statements reactive in legacy mode if needed
160160
if (!analysis.runes) {
161161
for (const [name, binding] of analysis.module.scope.declarations) {
162-
if (binding.kind === 'state' && binding.declaration_kind === 'import') {
162+
if (binding.kind === 'legacy_reactive_import') {
163163
instance.body.unshift(
164164
b.var('$$_import_' + name, b.call('$.reactive_import', b.thunk(b.id(name))))
165165
);
@@ -175,7 +175,7 @@ export function client_component(source, analysis, options) {
175175

176176
for (const [name, binding] of analysis.instance.scope.declarations) {
177177
if (binding.kind === 'legacy_reactive') {
178-
legacy_reactive_declarations.push(b.const(name, b.call('$.source')));
178+
legacy_reactive_declarations.push(b.const(name, b.call('$.mutable_source')));
179179
}
180180
if (binding.kind === 'store_sub') {
181181
if (store_setup.length === 0) {
@@ -240,11 +240,12 @@ export function client_component(source, analysis, options) {
240240

241241
const properties = analysis.exports.map(({ name, alias }) => {
242242
const binding = analysis.instance.scope.get(name);
243+
const is_source =
244+
binding?.kind === 'state' && (!state.analysis.immutable || binding.reassigned);
245+
243246
// TODO This is always a getter because the `renamed-instance-exports` test wants it that way.
244247
// Should we for code size reasons make it an init in runes mode and/or non-dev mode?
245-
return b.get(alias ?? name, [
246-
b.return(binding?.kind === 'state' ? b.call('$.get', b.id(name)) : b.id(name))
247-
]);
248+
return b.get(alias ?? name, [b.return(is_source ? b.call('$.get', b.id(name)) : b.id(name))]);
248249
});
249250

250251
if (analysis.accessors) {
@@ -261,14 +262,7 @@ export function client_component(source, analysis, options) {
261262
}
262263

263264
const component_block = b.block([
264-
b.stmt(
265-
b.call(
266-
'$.push',
267-
b.id('$$props'),
268-
b.literal(analysis.runes),
269-
...(options.immutable ? [b.literal(true)] : [])
270-
)
271-
),
265+
b.stmt(b.call('$.push', b.id('$$props'), b.literal(analysis.runes))),
272266
...store_setup,
273267
...legacy_reactive_declarations,
274268
...group_binding_declarations,

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

+70-23
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as b from '../../../utils/builders.js';
2-
import { extract_paths } from '../../../utils/ast.js';
2+
import { extract_paths, is_simple_expression } from '../../../utils/ast.js';
33
import { error } from '../../../errors.js';
44

55
/**
@@ -67,19 +67,20 @@ export function serialize_get_binding(node, state) {
6767

6868
if (
6969
binding.kind === 'prop' &&
70-
!binding.mutated &&
70+
!(state.analysis.immutable ? binding.reassigned : binding.mutated) &&
7171
!binding.initial &&
7272
!state.analysis.accessors
7373
) {
7474
return b.call(node);
7575
}
7676

77-
if (binding.kind === 'state' && binding.declaration_kind === 'import') {
77+
if (binding.kind === 'legacy_reactive_import') {
7878
return b.call('$$_import_' + node.name);
7979
}
8080

8181
if (
82-
binding.kind === 'state' ||
82+
(binding.kind === 'state' &&
83+
(!state.analysis.immutable || state.analysis.accessors || binding.reassigned)) ||
8384
binding.kind === 'derived' ||
8485
binding.kind === 'prop' ||
8586
binding.kind === 'rest_prop' ||
@@ -99,7 +100,7 @@ export function serialize_get_binding(node, state) {
99100
* @returns {import('estree').Expression}
100101
*/
101102
export function serialize_set_binding(node, context, fallback) {
102-
const { state, visit, path } = context;
103+
const { state, visit } = context;
103104

104105
if (
105106
node.left.type === 'ArrayPattern' ||
@@ -152,7 +153,11 @@ export function serialize_set_binding(node, context, fallback) {
152153
if (left.object.type === 'ThisExpression' && left.property.type === 'PrivateIdentifier') {
153154
if (context.state.private_state.has(left.property.name) && !state.in_constructor) {
154155
const value = get_assignment_value(node, context);
155-
return b.call('$.set', left, value);
156+
return b.call(
157+
'$.set',
158+
left,
159+
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
160+
);
156161
}
157162
}
158163
// @ts-expect-error
@@ -171,7 +176,7 @@ export function serialize_set_binding(node, context, fallback) {
171176
return binding.mutation(node, context);
172177
}
173178

174-
if (binding.kind === 'state' && binding.declaration_kind === 'import') {
179+
if (binding.kind === 'legacy_reactive_import') {
175180
return b.call(
176181
'$$_import_' + binding.node.name,
177182
b.assignment(
@@ -202,7 +207,11 @@ export function serialize_set_binding(node, context, fallback) {
202207
if (is_store) {
203208
return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value);
204209
} else {
205-
return b.call('$.set', b.id(left_name), value);
210+
return b.call(
211+
'$.set',
212+
b.id(left_name),
213+
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
214+
);
206215
}
207216
} else {
208217
if (is_store) {
@@ -216,7 +225,7 @@ export function serialize_set_binding(node, context, fallback) {
216225
),
217226
b.call('$' + left_name)
218227
);
219-
} else {
228+
} else if (!state.analysis.runes) {
220229
return b.call(
221230
'$.mutate',
222231
b.id(left_name),
@@ -226,6 +235,12 @@ export function serialize_set_binding(node, context, fallback) {
226235
value
227236
)
228237
);
238+
} else {
239+
return b.assignment(
240+
node.operator,
241+
/** @type {import('estree').Pattern} */ (visit(node.left)),
242+
/** @type {import('estree').Expression} */ (visit(node.right))
243+
);
229244
}
230245
}
231246
};
@@ -335,24 +350,38 @@ export function get_props_method(binding, state, name, default_value) {
335350
/** @type {import('estree').Expression[]} */
336351
const args = [b.id('$$props'), b.literal(name)];
337352

353+
// Use $.prop_source in the following cases:
354+
// - accessors/mutated: needs to be able to set the prop value from within
355+
// - default value: we set the fallback value only initially, and it's not possible to know this timing in $.prop
356+
const needs_source =
357+
default_value ||
358+
state.analysis.accessors ||
359+
(state.analysis.immutable ? binding.reassigned : binding.mutated);
360+
361+
if (needs_source) {
362+
args.push(b.literal(state.analysis.immutable));
363+
}
364+
338365
if (default_value) {
339366
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
340-
if (default_value.type !== 'Literal' && default_value.type !== 'Identifier') {
341-
args.push(b.thunk(default_value));
342-
args.push(b.true);
343-
} else {
367+
if (is_simple_expression(default_value)) {
344368
args.push(default_value);
345-
args.push(b.false);
369+
} else {
370+
if (
371+
default_value.type === 'CallExpression' &&
372+
default_value.callee.type === 'Identifier' &&
373+
default_value.arguments.length === 0
374+
) {
375+
args.push(default_value.callee);
376+
} else {
377+
args.push(b.thunk(default_value));
378+
}
379+
380+
args.push(b.true);
346381
}
347382
}
348383

349-
return b.call(
350-
// Use $.prop_source in the following cases:
351-
// - accessors/mutated: needs to be able to set the prop value from within
352-
// - default value: we set the fallback value only initially, and it's not possible to know this timing in $.prop
353-
binding.mutated || binding.initial || state.analysis.accessors ? '$.prop_source' : '$.prop',
354-
...args
355-
);
384+
return b.call(needs_source ? '$.prop_source' : '$.prop', ...args);
356385
}
357386

358387
/**
@@ -364,7 +393,7 @@ export function get_props_method(binding, state, name, default_value) {
364393
export function create_state_declarators(declarator, scope, value) {
365394
// in the simple `let count = $state(0)` case, we rewrite `$state` as `$.source`
366395
if (declarator.id.type === 'Identifier') {
367-
return [b.declarator(declarator.id, b.call('$.source', value))];
396+
return [b.declarator(declarator.id, b.call('$.mutable_source', value))];
368397
}
369398

370399
const tmp = scope.generate('tmp');
@@ -374,7 +403,25 @@ export function create_state_declarators(declarator, scope, value) {
374403
...paths.map((path) => {
375404
const value = path.expression?.(b.id(tmp));
376405
const binding = scope.get(/** @type {import('estree').Identifier} */ (path.node).name);
377-
return b.declarator(path.node, binding?.kind === 'state' ? b.call('$.source', value) : value);
406+
return b.declarator(
407+
path.node,
408+
binding?.kind === 'state' ? b.call('$.mutable_source', value) : value
409+
);
378410
})
379411
];
380412
}
413+
414+
/** @param {import('estree').Expression} node */
415+
export function should_proxy(node) {
416+
if (
417+
!node ||
418+
node.type === 'Literal' ||
419+
node.type === 'ArrowFunctionExpression' ||
420+
node.type === 'FunctionExpression' ||
421+
(node.type === 'Identifier' && node.name === 'undefined')
422+
) {
423+
return false;
424+
}
425+
426+
return true;
427+
}

packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js

+19-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { get_rune } from '../../../scope.js';
22
import { is_hoistable_function } from '../../utils.js';
33
import * as b from '../../../../utils/builders.js';
44
import * as assert from '../../../../utils/assert.js';
5-
import { create_state_declarators, get_props_method } from '../utils.js';
5+
import { create_state_declarators, get_props_method, should_proxy } from '../utils.js';
66
import { unwrap_ts_expression } from '../../../../utils/ast.js';
77

88
/** @type {import('../types.js').ComponentVisitors} */
@@ -84,7 +84,7 @@ export const javascript_visitors_runes = {
8484

8585
value =
8686
field.kind === 'state'
87-
? b.call('$.source', init)
87+
? b.call('$.source', should_proxy(init) ? b.call('$.proxy', init) : init)
8888
: b.call('$.derived', b.thunk(init));
8989
} else {
9090
// if no arguments, we know it's state as `$derived()` is a compile error
@@ -211,16 +211,28 @@ export const javascript_visitors_runes = {
211211
}
212212

213213
const args = /** @type {import('estree').CallExpression} */ (init).arguments;
214-
const value =
214+
let value =
215215
args.length === 0
216216
? b.id('undefined')
217217
: /** @type {import('estree').Expression} */ (visit(args[0]));
218-
const opts = args[1] && /** @type {import('estree').Expression} */ (visit(args[1]));
219218

220219
if (declarator.id.type === 'Identifier') {
221-
const callee = rune === '$state' ? '$.source' : '$.derived';
222-
const arg = rune === '$state' ? value : b.thunk(value);
223-
declarations.push(b.declarator(declarator.id, b.call(callee, arg, opts)));
220+
if (rune === '$state') {
221+
const binding = /** @type {import('#compiler').Binding} */ (
222+
state.scope.get(declarator.id.name)
223+
);
224+
if (should_proxy(value)) {
225+
value = b.call('$.proxy', value);
226+
}
227+
228+
if (!state.analysis.immutable || state.analysis.accessors || binding.reassigned) {
229+
value = b.call('$.source', value);
230+
}
231+
} else {
232+
value = b.call('$.derived', b.thunk(value));
233+
}
234+
235+
declarations.push(b.declarator(declarator.id, value));
224236
continue;
225237
}
226238

0 commit comments

Comments
 (0)