Skip to content

feat: allow let props = $props(), optimize prop read access #12201

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

Merged
merged 1 commit into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/six-gorillas-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: allow `let props = $props()` and optimize prop read access
65 changes: 37 additions & 28 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -1240,25 +1240,27 @@ 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);
}

if (state.scope !== state.analysis.instance.scope) {
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);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this code here instead of inside serialize_get_binding to not bloat that function even further and to not having to adjust the other callsites (AFAIK we only need that logic here)


return serialize_get_binding(node, state);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,53 +238,65 @@ export const javascript_visitors_runes = {
}

if (rune === '$props') {
assert.equal(declarator.id.type, 'ObjectPattern');

/** @type {string[]} */
const seen = ['$$slots', '$$events', '$$legacy'];

if (state.analysis.custom_element) {
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)));
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const rest_props_handler = {
* @param {string} [name]
* @returns {Record<string, unknown>}
*/
/*#__NO_SIDE_EFFECTS__*/
export function rest_props(props, exclude, name) {
return new Proxy(
DEV ? { props, exclude, name, other: {}, to_proxy: [] } : { props, exclude },
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
let props = $props();
props.a;
props[a];
props.a.b;
props.a.b = true;
props.a = true;
props[a] = true;
props;
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading