From 33e44ea697356ea3f21fc6649919cf7fb4948b22 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 28 Jun 2024 09:20:41 +0200 Subject: [PATCH] feat: allow `let props = $props()`, optimize prop read access (#12201) - allow to write `let props = $props()` - optimize read access of props.x to use `$$props` argument directly; closes #11055 --- .changeset/six-gorillas-obey.md | 5 ++ .../src/compiler/phases/2-analyze/index.js | 65 ++++++++------ .../compiler/phases/2-analyze/validation.js | 22 ++--- .../3-transform/client/visitors/global.js | 21 +++++ .../client/visitors/javascript-runes.js | 86 +++++++++++-------- .../src/internal/client/reactivity/props.js | 1 + .../_expected/client/index.svelte.js | 17 ++++ .../_expected/server/index.svelte.js | 16 ++++ .../samples/props-identifier/index.svelte | 10 +++ .../routes/docs/content/01-api/02-runes.md | 6 ++ 10 files changed, 174 insertions(+), 75 deletions(-) create mode 100644 .changeset/six-gorillas-obey.md create mode 100644 packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/props-identifier/index.svelte diff --git a/.changeset/six-gorillas-obey.md b/.changeset/six-gorillas-obey.md new file mode 100644 index 0000000000..258505a381 --- /dev/null +++ b/.changeset/six-gorillas-obey.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: allow `let props = $props()` and optimize prop read access diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 2e46d18c6f..f0f0178144 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -31,6 +31,7 @@ import { hash } from './utils.js'; import { warn_unused } from './css/css-warn.js'; import { extract_svelte_ignore } from '../../utils/extract_svelte_ignore.js'; import { ignore_map, ignore_stack, pop_ignore, push_ignore } from '../../state.js'; +import { equal } from '../../utils/assert.js'; /** * @param {import('#compiler').Script | null} script @@ -969,34 +970,42 @@ const runes_scope_tweaker = { if (rune === '$props') { state.analysis.needs_props = true; - for (const property of /** @type {import('estree').ObjectPattern} */ (node.id).properties) { - if (property.type !== 'Property') continue; - - const name = - property.value.type === 'AssignmentPattern' - ? /** @type {import('estree').Identifier} */ (property.value.left).name - : /** @type {import('estree').Identifier} */ (property.value).name; - const alias = - property.key.type === 'Identifier' - ? property.key.name - : String(/** @type {import('estree').Literal} */ (property.key).value); - let initial = property.value.type === 'AssignmentPattern' ? property.value.right : null; - - const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(name)); - binding.prop_alias = alias; - - // rewire initial from $props() to the actual initial value, stripping $bindable() if necessary - if ( - initial?.type === 'CallExpression' && - initial.callee.type === 'Identifier' && - initial.callee.name === '$bindable' - ) { - binding.initial = /** @type {import('estree').Expression | null} */ ( - initial.arguments[0] ?? null - ); - binding.kind = 'bindable_prop'; - } else { - binding.initial = initial; + if (node.id.type === 'Identifier') { + const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(node.id.name)); + binding.initial = null; // else would be $props() + binding.kind = 'rest_prop'; + } else { + equal(node.id.type, 'ObjectPattern'); + + for (const property of node.id.properties) { + if (property.type !== 'Property') continue; + + const name = + property.value.type === 'AssignmentPattern' + ? /** @type {import('estree').Identifier} */ (property.value.left).name + : /** @type {import('estree').Identifier} */ (property.value).name; + const alias = + property.key.type === 'Identifier' + ? property.key.name + : String(/** @type {import('estree').Literal} */ (property.key).value); + let initial = property.value.type === 'AssignmentPattern' ? property.value.right : null; + + const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(name)); + binding.prop_alias = alias; + + // rewire initial from $props() to the actual initial value, stripping $bindable() if necessary + if ( + initial?.type === 'CallExpression' && + initial.callee.type === 'Identifier' && + initial.callee.name === '$bindable' + ) { + binding.initial = /** @type {import('estree').Expression | null} */ ( + initial.arguments[0] ?? null + ); + binding.kind = 'bindable_prop'; + } else { + binding.initial = initial; + } } } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index f4778e4322..45f87935e1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -1240,7 +1240,7 @@ export const validation_runes = merge(validation, a11y_validators, { e.rune_invalid_arguments(node, rune); } - if (node.id.type !== 'ObjectPattern') { + if (node.id.type !== 'ObjectPattern' && node.id.type !== 'Identifier') { e.props_invalid_identifier(node); } @@ -1248,17 +1248,19 @@ export const validation_runes = merge(validation, a11y_validators, { e.props_invalid_placement(node); } - for (const property of node.id.properties) { - if (property.type === 'Property') { - if (property.computed) { - e.props_invalid_pattern(property); - } + if (node.id.type === 'ObjectPattern') { + for (const property of node.id.properties) { + if (property.type === 'Property') { + if (property.computed) { + e.props_invalid_pattern(property); + } - const value = - property.value.type === 'AssignmentPattern' ? property.value.left : property.value; + const value = + property.value.type === 'AssignmentPattern' ? property.value.left : property.value; - if (value.type !== 'Identifier') { - e.props_invalid_pattern(property); + if (value.type !== 'Identifier') { + e.props_invalid_pattern(property); + } } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index 37dd02855f..a640fb01b0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -9,6 +9,27 @@ export const global_visitors = { if (node.name === '$$props') { return b.id('$$sanitized_props'); } + + // Optimize prop access: If it's a member read access, we can use the $$props object directly + const binding = state.scope.get(node.name); + if ( + state.analysis.runes && // can't do this in legacy mode because the proxy does more than just read/write + binding !== null && + node !== binding.node && + binding.kind === 'rest_prop' + ) { + const parent = path.at(-1); + const grand_parent = path.at(-2); + if ( + parent?.type === 'MemberExpression' && + !parent.computed && + grand_parent?.type !== 'AssignmentExpression' && + grand_parent?.type !== 'UpdateExpression' + ) { + return b.id('$$props'); + } + } + return serialize_get_binding(node, state); } }, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 91934738b0..4588f71d47 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -238,8 +238,6 @@ export const javascript_visitors_runes = { } if (rune === '$props') { - assert.equal(declarator.id.type, 'ObjectPattern'); - /** @type {string[]} */ const seen = ['$$slots', '$$events', '$$legacy']; @@ -247,44 +245,58 @@ export const javascript_visitors_runes = { seen.push('$$host'); } - for (const property of declarator.id.properties) { - if (property.type === 'Property') { - const key = /** @type {import('estree').Identifier | import('estree').Literal} */ ( - property.key - ); - const name = key.type === 'Identifier' ? key.name : /** @type {string} */ (key.value); - - seen.push(name); - - let id = - property.value.type === 'AssignmentPattern' ? property.value.left : property.value; - assert.equal(id.type, 'Identifier'); - const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); - let initial = - binding.initial && - /** @type {import('estree').Expression} */ (visit(binding.initial)); - // We're adding proxy here on demand and not within the prop runtime function so that - // people not using proxied state anywhere in their code don't have to pay the additional bundle size cost - if (initial && binding.mutated && should_proxy_or_freeze(initial, state.scope)) { - initial = b.call('$.proxy', initial); - } + if (declarator.id.type === 'Identifier') { + /** @type {import('estree').Expression[]} */ + const args = [b.id('$$props'), b.array(seen.map((name) => b.literal(name)))]; - if (is_prop_source(binding, state)) { - declarations.push(b.declarator(id, get_prop_source(binding, state, name, initial))); - } - } else { - // RestElement - /** @type {import('estree').Expression[]} */ - const args = [b.id('$$props'), b.array(seen.map((name) => b.literal(name)))]; - - if (state.options.dev) { - // include rest name, so we can provide informative error messages - args.push( - b.literal(/** @type {import('estree').Identifier} */ (property.argument).name) + if (state.options.dev) { + // include rest name, so we can provide informative error messages + args.push(b.literal(declarator.id.name)); + } + + declarations.push(b.declarator(declarator.id, b.call('$.rest_props', ...args))); + } else { + assert.equal(declarator.id.type, 'ObjectPattern'); + + for (const property of declarator.id.properties) { + if (property.type === 'Property') { + const key = /** @type {import('estree').Identifier | import('estree').Literal} */ ( + property.key ); + const name = key.type === 'Identifier' ? key.name : /** @type {string} */ (key.value); + + seen.push(name); + + let id = + property.value.type === 'AssignmentPattern' ? property.value.left : property.value; + assert.equal(id.type, 'Identifier'); + const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); + let initial = + binding.initial && + /** @type {import('estree').Expression} */ (visit(binding.initial)); + // We're adding proxy here on demand and not within the prop runtime function so that + // people not using proxied state anywhere in their code don't have to pay the additional bundle size cost + if (initial && binding.mutated && should_proxy_or_freeze(initial, state.scope)) { + initial = b.call('$.proxy', initial); + } + + if (is_prop_source(binding, state)) { + declarations.push(b.declarator(id, get_prop_source(binding, state, name, initial))); + } + } else { + // RestElement + /** @type {import('estree').Expression[]} */ + const args = [b.id('$$props'), b.array(seen.map((name) => b.literal(name)))]; + + if (state.options.dev) { + // include rest name, so we can provide informative error messages + args.push( + b.literal(/** @type {import('estree').Identifier} */ (property.argument).name) + ); + } + + declarations.push(b.declarator(property.argument, b.call('$.rest_props', ...args))); } - - declarations.push(b.declarator(property.argument, b.call('$.rest_props', ...args))); } } diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 3232239a75..c6b73ee6b5 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -78,6 +78,7 @@ const rest_props_handler = { * @param {string} [name] * @returns {Record} */ +/*#__NO_SIDE_EFFECTS__*/ export function rest_props(props, exclude, name) { return new Proxy( DEV ? { props, exclude, name, other: {}, to_proxy: [] } : { props, exclude }, diff --git a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js new file mode 100644 index 0000000000..2a10dbc1b1 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js @@ -0,0 +1,17 @@ +import "svelte/internal/disclose-version"; +import * as $ from "svelte/internal/client"; + +export default function Props_identifier($$anchor, $$props) { + $.push($$props, true); + + let props = $.rest_props($$props, ["$$slots", "$$events", "$$legacy"]); + + $$props.a; + props[a]; + $$props.a.b; + $$props.a.b = true; + props.a = true; + props[a] = true; + props; + $.pop(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js new file mode 100644 index 0000000000..362d773be1 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js @@ -0,0 +1,16 @@ +import * as $ from "svelte/internal/server"; + +export default function Props_identifier($$payload, $$props) { + $.push(); + + let props = $$props; + + props.a; + props[a]; + props.a.b; + props.a.b = true; + props.a = true; + props[a] = true; + props; + $.pop(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/props-identifier/index.svelte b/packages/svelte/tests/snapshot/samples/props-identifier/index.svelte new file mode 100644 index 0000000000..ebd9d09dca --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/props-identifier/index.svelte @@ -0,0 +1,10 @@ + diff --git a/sites/svelte-5-preview/src/routes/docs/content/01-api/02-runes.md b/sites/svelte-5-preview/src/routes/docs/content/01-api/02-runes.md index 84010d4f9b..06b4f97834 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/01-api/02-runes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/01-api/02-runes.md @@ -548,6 +548,12 @@ To get all properties, use rest syntax: let { a, b, c, ...everythingElse } = $props(); ``` +You can also use an identifier: + +```js +let props = $props(); +``` + If you're using TypeScript, you can declare the prop types: ```ts