Skip to content

Commit 1e4af19

Browse files
Rich-HarrisRich-Harris
authored
chore: use $$props directly where possible (#9813)
* use $$props directly in runes mode * this makes no sense * use $$props directly in runes mode * tidy up * typo * remove unreachable code --------- Co-authored-by: Rich Harris <[email protected]>
1 parent 074615d commit 1e4af19

File tree

5 files changed

+80
-107
lines changed

5 files changed

+80
-107
lines changed

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

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,20 @@ export function serialize_get_binding(node, state) {
6565
return binding.expression;
6666
}
6767

68-
if (binding.kind === 'prop' && binding.node.name === '$$props') {
69-
// Special case for $$props which only exists in the old world
70-
return node;
71-
}
68+
if (binding.kind === 'prop') {
69+
if (binding.node.name === '$$props') {
70+
// Special case for $$props which only exists in the old world
71+
// TODO this probably shouldn't have a 'prop' binding kind
72+
return node;
73+
}
7274

73-
if (
74-
binding.kind === 'prop' &&
75-
!(state.analysis.immutable ? binding.reassigned : binding.mutated) &&
76-
!binding.initial &&
77-
!state.analysis.accessors
78-
) {
79-
return b.call(node);
75+
if (
76+
!state.analysis.accessors &&
77+
!(state.analysis.immutable ? binding.reassigned : binding.mutated) &&
78+
!binding.initial
79+
) {
80+
return b.member(b.id('$$props'), node);
81+
}
8082
}
8183

8284
if (binding.kind === 'legacy_reactive_import') {
@@ -343,67 +345,53 @@ export function serialize_hoistable_params(node, context) {
343345
}
344346

345347
/**
346-
*
347-
* @param {import('#compiler').Binding} binding
348348
* @param {import('./types').ComponentClientTransformState} state
349349
* @param {string} name
350-
* @param {import('estree').Expression | null} [default_value]
350+
* @param {import('estree').Expression | null} [initial]
351351
* @returns
352352
*/
353-
export function get_props_method(binding, state, name, default_value) {
353+
export function get_prop_source(state, name, initial) {
354354
/** @type {import('estree').Expression[]} */
355355
const args = [b.id('$$props'), b.literal(name)];
356356

357-
// Use $.prop_source in the following cases:
358-
// - accessors/mutated: needs to be able to set the prop value from within
359-
// - default value: we set the fallback value only initially, and it's not possible to know this timing in $.prop
360-
const needs_source =
361-
default_value ||
362-
state.analysis.accessors ||
363-
(state.analysis.immutable ? binding.reassigned : binding.mutated);
364-
365-
if (needs_source) {
366-
let flags = 0;
357+
let flags = 0;
367358

368-
/** @type {import('estree').Expression | undefined} */
369-
let arg;
359+
if (state.analysis.immutable) {
360+
flags |= PROPS_IS_IMMUTABLE;
361+
}
370362

371-
if (state.analysis.immutable) {
372-
flags |= PROPS_IS_IMMUTABLE;
373-
}
363+
if (state.analysis.runes) {
364+
flags |= PROPS_IS_RUNES;
365+
}
374366

375-
if (state.analysis.runes) {
376-
flags |= PROPS_IS_RUNES;
377-
}
367+
/** @type {import('estree').Expression | undefined} */
368+
let arg;
378369

379-
if (default_value) {
380-
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
381-
if (is_simple_expression(default_value)) {
382-
arg = default_value;
370+
if (initial) {
371+
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
372+
if (is_simple_expression(initial)) {
373+
arg = initial;
374+
} else {
375+
if (
376+
initial.type === 'CallExpression' &&
377+
initial.callee.type === 'Identifier' &&
378+
initial.arguments.length === 0
379+
) {
380+
arg = initial.callee;
383381
} else {
384-
if (
385-
default_value.type === 'CallExpression' &&
386-
default_value.callee.type === 'Identifier' &&
387-
default_value.arguments.length === 0
388-
) {
389-
arg = default_value.callee;
390-
} else {
391-
arg = b.thunk(default_value);
392-
}
393-
394-
flags |= PROPS_CALL_DEFAULT_VALUE;
382+
arg = b.thunk(initial);
395383
}
396-
}
397384

398-
if (flags || arg) {
399-
args.push(b.literal(flags));
400-
if (arg) args.push(arg);
385+
flags |= PROPS_CALL_DEFAULT_VALUE;
401386
}
387+
}
402388

403-
return b.call('$.prop_source', ...args);
389+
if (flags || arg) {
390+
args.push(b.literal(flags));
391+
if (arg) args.push(arg);
404392
}
405393

406-
return b.call('$.prop', ...args);
394+
return b.call('$.prop_source', ...args);
407395
}
408396

409397
/**

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { is_hoistable_function } from '../../utils.js';
22
import * as b from '../../../../utils/builders.js';
33
import { extract_paths } from '../../../../utils/ast.js';
4-
import { create_state_declarators, get_props_method, serialize_get_binding } from '../utils.js';
4+
import { create_state_declarators, get_prop_source, serialize_get_binding } from '../utils.js';
55

66
/** @type {import('../types.js').ComponentVisitors} */
77
export const javascript_visitors_legacy = {
@@ -54,8 +54,8 @@ export const javascript_visitors_legacy = {
5454
declarations.push(
5555
b.declarator(
5656
path.node,
57-
binding.kind === 'prop' || binding.kind === 'rest_prop'
58-
? get_props_method(binding, state, binding.prop_alias ?? name, value)
57+
binding.kind === 'prop'
58+
? get_prop_source(state, binding.prop_alias ?? name, value)
5959
: value
6060
)
6161
);
@@ -67,17 +67,23 @@ export const javascript_visitors_legacy = {
6767
state.scope.get(declarator.id.name)
6868
);
6969

70-
declarations.push(
71-
b.declarator(
72-
declarator.id,
73-
get_props_method(
74-
binding,
75-
state,
76-
binding.prop_alias ?? declarator.id.name,
77-
declarator.init && /** @type {import('estree').Expression} */ (visit(declarator.init))
70+
if (
71+
state.analysis.accessors ||
72+
(state.analysis.immutable ? binding.reassigned : binding.mutated) ||
73+
declarator.init
74+
) {
75+
declarations.push(
76+
b.declarator(
77+
declarator.id,
78+
get_prop_source(
79+
state,
80+
binding.prop_alias ?? declarator.id.name,
81+
declarator.init &&
82+
/** @type {import('estree').Expression} */ (visit(declarator.init))
83+
)
7884
)
79-
)
80-
);
85+
);
86+
}
8187

8288
continue;
8389
}

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

Lines changed: 19 additions & 28 deletions
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, should_proxy } from '../utils.js';
5+
import { create_state_declarators, get_prop_source, should_proxy } from '../utils.js';
66
import { unwrap_ts_expression } from '../../../../utils/ast.js';
77

88
/** @type {import('../types.js').ComponentVisitors} */
@@ -165,36 +165,27 @@ export const javascript_visitors_runes = {
165165

166166
for (const property of declarator.id.properties) {
167167
if (property.type === 'Property') {
168-
assert.ok(property.key.type === 'Identifier' || property.key.type === 'Literal');
169-
let name;
170-
if (property.key.type === 'Identifier') {
171-
name = property.key.name;
172-
} else if (property.key.type === 'Literal') {
173-
name = /** @type {string} */ (property.key.value).toString();
174-
} else {
175-
throw new Error('unreachable');
176-
}
168+
const key = /** @type {import('estree').Identifier | import('estree').Literal} */ (
169+
property.key
170+
);
171+
const name = key.type === 'Identifier' ? key.name : /** @type {string} */ (key.value);
177172

178173
seen.push(name);
179174

180-
if (property.value.type === 'Identifier') {
181-
const binding = /** @type {import('#compiler').Binding} */ (
182-
state.scope.get(property.value.name)
183-
);
184-
declarations.push(
185-
b.declarator(property.value, get_props_method(binding, state, name))
186-
);
187-
} else if (property.value.type === 'AssignmentPattern') {
188-
assert.equal(property.value.left.type, 'Identifier');
189-
const binding = /** @type {import('#compiler').Binding} */ (
190-
state.scope.get(property.value.left.name)
191-
);
192-
declarations.push(
193-
b.declarator(
194-
property.value.left,
195-
get_props_method(binding, state, name, property.value.right)
196-
)
197-
);
175+
let id = property.value;
176+
let initial = undefined;
177+
178+
if (property.value.type === 'AssignmentPattern') {
179+
id = property.value.left;
180+
initial = property.value.right;
181+
}
182+
183+
assert.equal(id.type, 'Identifier');
184+
185+
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name));
186+
187+
if (binding.reassigned || state.analysis.accessors || initial) {
188+
declarations.push(b.declarator(id, get_prop_source(state, name, initial)));
198189
}
199190
} else {
200191
// RestElement

‎packages/svelte/src/internal/client/runtime.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,17 +1490,6 @@ export function prop_source(props, key, flags, default_value) {
14901490
return /** @type {import('./types.js').Signal<V>} */ (source_signal);
14911491
}
14921492

1493-
/**
1494-
* If the prop is readonly and has no fallback value, we can use this function, else we need to use `prop_source`.
1495-
* @param {Record<string, unknown>} props
1496-
* @param {string} key
1497-
* @returns {any}
1498-
*/
1499-
export function prop(props, key) {
1500-
// TODO skip this, and rewrite as `$$props.foo`
1501-
return () => props[key];
1502-
}
1503-
15041493
/**
15051494
* @param {boolean} immutable
15061495
* @param {unknown} a

‎packages/svelte/src/internal/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ export {
77
source,
88
mutable_source,
99
derived,
10-
prop,
1110
prop_source,
1211
user_effect,
1312
render_effect,

0 commit comments

Comments
 (0)