Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxied state #9739

Merged
merged 70 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
fa8ca04
magic objects
Rich-Harris Nov 28, 2023
b6fb020
read length eagerly — triggers reconciliation
Rich-Harris Nov 28, 2023
f61370e
nested magic
Rich-Harris Nov 28, 2023
b45cd60
tests
Rich-Harris Nov 28, 2023
9bf3a6d
more tests
Rich-Harris Nov 28, 2023
a77eca2
fix array memory leak
Rich-Harris Nov 28, 2023
575d0fc
skipped, partially passing array test
Rich-Harris Nov 28, 2023
80b2253
Fix each
trueadm Nov 29, 2023
3a6ad8d
more 1337
Rich-Harris Nov 29, 2023
0db3edc
eliminate closures
Rich-Harris Nov 29, 2023
3b754b5
maybe this is unnecessary?
Rich-Harris Nov 29, 2023
44d0087
only create sources for own properties
Rich-Harris Nov 29, 2023
fcfce74
more
Rich-Harris Nov 29, 2023
fb6043e
rename stuff
Rich-Harris Nov 29, 2023
39214ed
shuffle things around
Rich-Harris Nov 29, 2023
03288f5
emit $.proxy
Rich-Harris Nov 29, 2023
ef6ee52
remove proxy helper from public API
Rich-Harris Nov 29, 2023
8a2e941
only create sources for writable properties
Rich-Harris Nov 29, 2023
5f8988a
update test
Rich-Harris Nov 29, 2023
5ff4932
get tests passing
Rich-Harris Nov 29, 2023
c59b301
fix
Rich-Harris Nov 29, 2023
6d151ce
remove state-not-mutated warning, which is no longer valid
Rich-Harris Nov 29, 2023
5a899c2
track reassignments separately from mutations
Rich-Harris Nov 29, 2023
0e1cdd9
update test - effects no longer fire on mutations unnecessarily
Rich-Harris Nov 29, 2023
358c938
move util into utils file
Rich-Harris Nov 29, 2023
c063a71
move each logic into its own module; breathe sigh of relief
Rich-Harris Nov 29, 2023
236b191
tweaks
trueadm Nov 30, 2023
b2ddc46
more tweaks
trueadm Nov 30, 2023
0982d19
improve runtime
trueadm Nov 30, 2023
cb731aa
fix mistake
trueadm Nov 30, 2023
4288843
ensure we proxy when assigning to state
trueadm Nov 30, 2023
28ac98e
fix test
trueadm Nov 30, 2023
1513642
handle frozen
trueadm Nov 30, 2023
e28095f
create sources when reading proxy properties inside deriveds
Rich-Harris Nov 30, 2023
b049cc7
only mutate in legacy mode
Rich-Harris Nov 30, 2023
afd2efa
add immutable to transform state
Rich-Harris Nov 30, 2023
84a30b5
remove unused second argument to derived
Rich-Harris Nov 30, 2023
ccc47a0
remove unused second argument to source, and runtime immutable option…
Rich-Harris Nov 30, 2023
1e2f6cb
oops, backwards
Rich-Harris Nov 30, 2023
c01fa5c
dedicated binding.kind for legacy reactive imports
Rich-Harris Nov 30, 2023
b581a64
avoid using prop_source in more cases. bit hacky, could be tidier, bu…
Rich-Harris Nov 30, 2023
08c069d
distinguish between source and mutable_source
Rich-Harris Dec 1, 2023
2347fd1
remove immutable option from mount
Rich-Harris Dec 1, 2023
95fa5ec
remove unused apparatus around immutable option
Rich-Harris Dec 1, 2023
78ae50a
deprecate immutable
Rich-Harris Dec 1, 2023
7fe00b2
fix
Rich-Harris Dec 1, 2023
0df903e
tweak
Rich-Harris Dec 1, 2023
7fc251d
better default value handling
Rich-Harris Dec 1, 2023
233f88f
remove unnecessary exports
Rich-Harris Dec 1, 2023
0acdecc
whitespace is free
Rich-Harris Dec 1, 2023
c8bc0b3
remove obsolete symbol from proxy
trueadm Dec 1, 2023
87150bb
cleanup ts
trueadm Dec 1, 2023
574d132
improve runtime perf by removing version reads in has()
trueadm Dec 1, 2023
676ef45
add missing proxy call
trueadm Dec 1, 2023
e2a6619
tune perf of has() more
trueadm Dec 1, 2023
1c92ddc
ensure we only create sources in effect_active()
trueadm Dec 1, 2023
468ee8b
fix proxy of computed fields
trueadm Dec 1, 2023
0cc90a7
simplify and fix issue with indexed each blocks
trueadm Dec 1, 2023
4b6e0eb
fix compiler errors around exported state
Rich-Harris Dec 4, 2023
2bc7c03
only create source for state that is reassigned
Rich-Harris Dec 4, 2023
28462a5
temporary fix, we should come up with something better than this
Rich-Harris Dec 4, 2023
0c9fad3
readonly props
Rich-Harris Dec 4, 2023
c7ed79c
fix test
Rich-Harris Dec 4, 2023
9c937e6
add test for bind:
Rich-Harris Dec 4, 2023
6539ad0
make readonly dev-only
Rich-Harris Dec 4, 2023
e6ac035
merge main
Rich-Harris Dec 4, 2023
1dbb855
docs
Rich-Harris Dec 4, 2023
4faca28
merge main
Rich-Harris Dec 4, 2023
608a3e7
forbid setPrototypeOf
Rich-Harris Dec 4, 2023
caa718e
lol whoops
Rich-Harris Dec 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ const runes = {
'invalid-legacy-export': () => `Cannot use \`export let\` in runes mode — use $props instead`,
/** @param {string} rune */
'invalid-rune-usage': (rune) => `Cannot use ${rune} rune in non-runes mode`,
/** @param {string} rune */
'invalid-rune-export': (rune) => `Cannot export value created with ${rune}`,
'invalid-state-export': () => `Cannot export state if it is reassigned`,
'invalid-derived-export': () => `Cannot export derived state`,
'invalid-props-id': () => `$props() can only be used with an object destructuring pattern`,
'invalid-props-pattern': () =>
`$props() assignment must not contain nested properties or computed keys`,
Expand Down
37 changes: 11 additions & 26 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function get_delegated_event(node, context) {
// or any normal not reactive bindings that are mutated.
binding.kind === 'normal' ||
// or any reactive imports (those are rewritten) (can only happen in legacy mode)
(binding.kind === 'state' && binding.declaration_kind === 'import')) &&
binding.kind === 'legacy_reactive_import') &&
binding.mutated))
) {
return non_hoistable;
Expand Down Expand Up @@ -221,20 +221,13 @@ export function analyze_module(ast, options) {
merge(set_scope(scopes), validation_runes_js, runes_scope_js_tweaker)
);

// If we are in runes mode, then check for possible misuses of state runes
for (const [, scope] of scopes) {
for (const [name, binding] of scope.declarations) {
if (binding.kind === 'state' && !binding.mutated) {
warn(warnings, binding.node, [], 'state-not-mutated', name);
}
}
}

return {
module: { ast, scope, scopes },
name: options.filename || 'module',
warnings,
accessors: false
accessors: false,
runes: true,
immutable: true
};
}

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

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

const runes =
options.runes ??
Array.from(module.scope.references).some(([name]) => Runes.includes(/** @type {any} */ (name)));

// TODO remove all the ?? stuff, we don't need it now that we're validating the config
/** @type {import('../types.js').ComponentAnalysis} */
const analysis = {
Expand All @@ -331,11 +328,8 @@ export function analyze_component(root, options) {
component_name,
get_css_hash: options.cssHash
}),
runes:
options.runes ??
Array.from(module.scope.references).some(([name]) =>
Runes.includes(/** @type {any} */ (name))
),
runes,
immutable: runes || options.immutable,
exports: [],
uses_props: false,
uses_rest_props: false,
Expand Down Expand Up @@ -382,15 +376,6 @@ export function analyze_component(root, options) {
merge(set_scope(scopes), validation_runes, runes_scope_tweaker, common_visitors)
);
}

// If we are in runes mode, then check for possible misuses of state runes
for (const [, scope] of instance.scopes) {
for (const [name, binding] of scope.declarations) {
if (binding.kind === 'state' && !binding.mutated) {
warn(warnings, binding.node, [], 'state-not-mutated', name);
}
}
}
} else {
instance.scope.declare(b.id('$$props'), 'prop', 'synthetic');
instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic');
Expand Down Expand Up @@ -553,7 +538,7 @@ const legacy_scope_tweaker = {
(state.reactive_statement || state.ast_type === 'template') &&
parent.type === 'MemberExpression'
) {
binding.kind = 'state';
binding.kind = 'legacy_reactive_import';
}
} else if (
binding.mutated &&
Expand Down
10 changes: 8 additions & 2 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,14 @@ export const validation_legacy = merge(validation, a11y_validators, {
*/
function validate_export(node, scope, name) {
const binding = scope.get(name);
if (binding && (binding.kind === 'derived' || binding.kind === 'state')) {
error(node, 'invalid-rune-export', `$${binding.kind}`);
if (!binding) return;

if (binding.kind === 'derived') {
error(node, 'invalid-derived-export');
}

if (binding.kind === 'state' && binding.reassigned) {
error(node, 'invalid-state-export');
}
}

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

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

const properties = analysis.exports.map(({ name, alias }) => {
const binding = analysis.instance.scope.get(name);
const is_source =
binding?.kind === 'state' && (!state.analysis.immutable || binding.reassigned);

// TODO This is always a getter because the `renamed-instance-exports` test wants it that way.
// Should we for code size reasons make it an init in runes mode and/or non-dev mode?
return b.get(alias ?? name, [
b.return(binding?.kind === 'state' ? b.call('$.get', b.id(name)) : b.id(name))
]);
return b.get(alias ?? name, [b.return(is_source ? b.call('$.get', b.id(name)) : b.id(name))]);
});

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

const component_block = b.block([
b.stmt(
b.call(
'$.push',
b.id('$$props'),
b.literal(analysis.runes),
...(options.immutable ? [b.literal(true)] : [])
)
),
b.stmt(b.call('$.push', b.id('$$props'), b.literal(analysis.runes))),
...store_setup,
...legacy_reactive_declarations,
...group_binding_declarations,
Expand Down
93 changes: 70 additions & 23 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as b from '../../../utils/builders.js';
import { extract_paths } from '../../../utils/ast.js';
import { extract_paths, is_simple_expression } from '../../../utils/ast.js';
import { error } from '../../../errors.js';

/**
Expand Down Expand Up @@ -67,19 +67,20 @@ export function serialize_get_binding(node, state) {

if (
binding.kind === 'prop' &&
!binding.mutated &&
!(state.analysis.immutable ? binding.reassigned : binding.mutated) &&
!binding.initial &&
!state.analysis.accessors
) {
return b.call(node);
}

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

if (
binding.kind === 'state' ||
(binding.kind === 'state' &&
(!state.analysis.immutable || state.analysis.accessors || binding.reassigned)) ||
binding.kind === 'derived' ||
binding.kind === 'prop' ||
binding.kind === 'rest_prop' ||
Expand All @@ -99,7 +100,7 @@ export function serialize_get_binding(node, state) {
* @returns {import('estree').Expression}
*/
export function serialize_set_binding(node, context, fallback) {
const { state, visit, path } = context;
const { state, visit } = context;

if (
node.left.type === 'ArrayPattern' ||
Expand Down Expand Up @@ -152,7 +153,11 @@ export function serialize_set_binding(node, context, fallback) {
if (left.object.type === 'ThisExpression' && left.property.type === 'PrivateIdentifier') {
if (context.state.private_state.has(left.property.name) && !state.in_constructor) {
const value = get_assignment_value(node, context);
return b.call('$.set', left, value);
return b.call(
'$.set',
left,
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
);
}
}
// @ts-expect-error
Expand All @@ -171,7 +176,7 @@ export function serialize_set_binding(node, context, fallback) {
return binding.mutation(node, context);
}

if (binding.kind === 'state' && binding.declaration_kind === 'import') {
if (binding.kind === 'legacy_reactive_import') {
return b.call(
'$$_import_' + binding.node.name,
b.assignment(
Expand Down Expand Up @@ -202,7 +207,11 @@ export function serialize_set_binding(node, context, fallback) {
if (is_store) {
return b.call('$.store_set', serialize_get_binding(b.id(left_name), state), value);
} else {
return b.call('$.set', b.id(left_name), value);
return b.call(
'$.set',
b.id(left_name),
context.state.analysis.runes && should_proxy(value) ? b.call('$.proxy', value) : value
);
}
} else {
if (is_store) {
Expand All @@ -216,7 +225,7 @@ export function serialize_set_binding(node, context, fallback) {
),
b.call('$' + left_name)
);
} else {
} else if (!state.analysis.runes) {
return b.call(
'$.mutate',
b.id(left_name),
Expand All @@ -226,6 +235,12 @@ export function serialize_set_binding(node, context, fallback) {
value
)
);
} else {
return b.assignment(
node.operator,
/** @type {import('estree').Pattern} */ (visit(node.left)),
/** @type {import('estree').Expression} */ (visit(node.right))
);
}
}
};
Expand Down Expand Up @@ -335,24 +350,38 @@ export function get_props_method(binding, state, name, default_value) {
/** @type {import('estree').Expression[]} */
const args = [b.id('$$props'), b.literal(name)];

// Use $.prop_source in the following cases:
// - accessors/mutated: needs to be able to set the prop value from within
// - default value: we set the fallback value only initially, and it's not possible to know this timing in $.prop
const needs_source =
default_value ||
state.analysis.accessors ||
(state.analysis.immutable ? binding.reassigned : binding.mutated);

if (needs_source) {
args.push(b.literal(state.analysis.immutable));
}

if (default_value) {
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
if (default_value.type !== 'Literal' && default_value.type !== 'Identifier') {
args.push(b.thunk(default_value));
args.push(b.true);
} else {
if (is_simple_expression(default_value)) {
args.push(default_value);
args.push(b.false);
} else {
if (
default_value.type === 'CallExpression' &&
default_value.callee.type === 'Identifier' &&
default_value.arguments.length === 0
) {
args.push(default_value.callee);
} else {
args.push(b.thunk(default_value));
}

args.push(b.true);
}
}

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

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

const tmp = scope.generate('tmp');
Expand All @@ -374,7 +403,25 @@ export function create_state_declarators(declarator, scope, value) {
...paths.map((path) => {
const value = path.expression?.(b.id(tmp));
const binding = scope.get(/** @type {import('estree').Identifier} */ (path.node).name);
return b.declarator(path.node, binding?.kind === 'state' ? b.call('$.source', value) : value);
return b.declarator(
path.node,
binding?.kind === 'state' ? b.call('$.mutable_source', value) : value
);
})
];
}

/** @param {import('estree').Expression} node */
export function should_proxy(node) {
if (
!node ||
node.type === 'Literal' ||
node.type === 'ArrowFunctionExpression' ||
node.type === 'FunctionExpression' ||
(node.type === 'Identifier' && node.name === 'undefined')
) {
return false;
}

return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { get_rune } from '../../../scope.js';
import { is_hoistable_function } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import { create_state_declarators, get_props_method } from '../utils.js';
import { create_state_declarators, get_props_method, should_proxy } from '../utils.js';
import { unwrap_ts_expression } from '../../../../utils/ast.js';

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

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

const args = /** @type {import('estree').CallExpression} */ (init).arguments;
const value =
let value =
args.length === 0
? b.id('undefined')
: /** @type {import('estree').Expression} */ (visit(args[0]));
const opts = args[1] && /** @type {import('estree').Expression} */ (visit(args[1]));

if (declarator.id.type === 'Identifier') {
const callee = rune === '$state' ? '$.source' : '$.derived';
const arg = rune === '$state' ? value : b.thunk(value);
declarations.push(b.declarator(declarator.id, b.call(callee, arg, opts)));
if (rune === '$state') {
const binding = /** @type {import('#compiler').Binding} */ (
state.scope.get(declarator.id.name)
);
if (should_proxy(value)) {
value = b.call('$.proxy', value);
}

if (!state.analysis.immutable || state.analysis.accessors || binding.reassigned) {
value = b.call('$.source', value);
}
} else {
value = b.call('$.derived', b.thunk(value));
}

declarations.push(b.declarator(declarator.id, value));
continue;
}

Expand Down
Loading