From d793d570e25c5fe6dde0980b96925b6fa274aa12 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 6 Dec 2023 15:48:33 +0000 Subject: [PATCH 1/7] fix: improve consistency issues around binding invalidation (#9810) * co * Add comment --- .changeset/soft-clocks-remember.md | 5 +++++ .../3-transform/client/visitors/template.js | 2 +- packages/svelte/src/internal/client/runtime.js | 11 +++++++++++ packages/svelte/src/internal/index.js | 1 + .../samples/invalidate-effect/_config.js | 16 ++++++++++++++++ .../samples/invalidate-effect/main.svelte | 13 +++++++++++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 .changeset/soft-clocks-remember.md create mode 100644 packages/svelte/tests/runtime-runes/samples/invalidate-effect/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/invalidate-effect/main.svelte diff --git a/.changeset/soft-clocks-remember.md b/.changeset/soft-clocks-remember.md new file mode 100644 index 0000000000..69e8aca06e --- /dev/null +++ b/.changeset/soft-clocks-remember.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: improve consistency issues around binding invalidation diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 6579d17c01..6e3e570454 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -245,7 +245,7 @@ function setup_select_synchronization(value_binding, context) { context.state.init.push( b.stmt( b.call( - '$.pre_effect', + '$.invalidate_effect', b.thunk( b.block([ b.stmt( diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 5d92c436e8..f075d2d5a3 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1273,6 +1273,17 @@ export function pre_effect(init) { ); } +/** + * This effect is used to ensure binding are kept in sync. We use a pre effect to ensure we run before the + * bindings which are in later effects. However, we don't use a pre_effect directly as we don't want to flush anything. + * + * @param {() => void | (() => void)} init + * @returns {import('./types.js').EffectSignal} + */ +export function invalidate_effect(init) { + return internal_create_effect(PRE_EFFECT, init, true, current_block, true); +} + /** * @param {() => void | (() => void)} init * @returns {import('./types.js').EffectSignal} diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 4d3205b824..a94a0de180 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -12,6 +12,7 @@ export { user_effect, render_effect, pre_effect, + invalidate_effect, flushSync, bubble_event, safe_equal, diff --git a/packages/svelte/tests/runtime-runes/samples/invalidate-effect/_config.js b/packages/svelte/tests/runtime-runes/samples/invalidate-effect/_config.js new file mode 100644 index 0000000000..b4ba660ad4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/invalidate-effect/_config.js @@ -0,0 +1,16 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + assert.htmlEqual(target.innerHTML, 'a\n From d5167e75b96e10f0ed8f9b84588079d66dffaec6 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 6 Dec 2023 16:04:01 +0000 Subject: [PATCH 2/7] fix: improve non state referenced warning (#9809) * fix: improve non state referenced warning * add test --- .changeset/polite-dolphins-care.md | 5 +++++ packages/svelte/src/compiler/phases/2-analyze/index.js | 4 ++-- .../samples/runes-referenced-nonstate-2/_config.js | 3 +++ .../samples/runes-referenced-nonstate-2/input.svelte | 6 ++++++ .../samples/runes-referenced-nonstate-2/warnings.json | 1 + 5 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 .changeset/polite-dolphins-care.md create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/_config.js create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/input.svelte create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/warnings.json diff --git a/.changeset/polite-dolphins-care.md b/.changeset/polite-dolphins-care.md new file mode 100644 index 0000000000..409b7e66d6 --- /dev/null +++ b/.changeset/polite-dolphins-care.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: improve non state referenced warning diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index be66f41d27..8d7d9a5906 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -409,10 +409,10 @@ export function analyze_component(root, options) { analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements); } - // warn on any nonstate declarations that are a) mutated and b) referenced in the template + // warn on any nonstate declarations that are a) reassigned and mutated and b) referenced in the template for (const scope of [module.scope, instance.scope]) { outer: for (const [name, binding] of scope.declarations) { - if (binding.kind === 'normal' && binding.mutated) { + if (binding.kind === 'normal' && binding.reassigned && binding.mutated) { for (const { path } of binding.references) { if (path[0].type !== 'Fragment') continue; for (let i = 1; i < path.length; i += 1) { diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/_config.js b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/_config.js new file mode 100644 index 0000000000..f47bee71df --- /dev/null +++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/input.svelte b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/input.svelte new file mode 100644 index 0000000000..42f1b675de --- /dev/null +++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/input.svelte @@ -0,0 +1,6 @@ + + + +

{JSON.stringify(a)} + {a.b}

diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/warnings.json b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/warnings.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate-2/warnings.json @@ -0,0 +1 @@ +[] From 56677859032d7950630f816fdcf6647f18b0e5af Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 6 Dec 2023 17:08:41 +0100 Subject: [PATCH 3/7] fix: better readonly checks for proxies (#9808) - Expect the thing that's checked to be wrapped with the proxy already, so that we can just check for the state symbol - Make error message more descriptive --- .changeset/five-tigers-search.md | 5 +++ .../svelte/src/internal/client/proxy/proxy.js | 2 +- .../src/internal/client/proxy/readonly.js | 35 +++++++++---------- .../svelte/src/internal/client/runtime.js | 4 +-- .../Counter.svelte | 8 +++++ .../_config.js | 22 ++++++++++++ .../main.svelte | 25 +++++++++++++ .../proxy-prop-default-readonly/_config.js | 3 +- .../samples/proxy-prop-readonly/_config.js | 3 +- 9 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 .changeset/five-tigers-search.md create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte diff --git a/.changeset/five-tigers-search.md b/.changeset/five-tigers-search.md new file mode 100644 index 0000000000..fb345c559f --- /dev/null +++ b/.changeset/five-tigers-search.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: better readonly checks for proxies diff --git a/packages/svelte/src/internal/client/proxy/proxy.js b/packages/svelte/src/internal/client/proxy/proxy.js index 695ec522c1..ac94c0b492 100644 --- a/packages/svelte/src/internal/client/proxy/proxy.js +++ b/packages/svelte/src/internal/client/proxy/proxy.js @@ -16,12 +16,12 @@ import { is_array, object_keys } from '../utils.js'; -import { READONLY_SYMBOL } from './readonly.js'; /** @typedef {{ s: Map>; v: import('../types.js').SourceSignal; a: boolean, i: boolean }} Metadata */ /** @typedef {Record & { [STATE_SYMBOL]: Metadata }} StateObject */ export const STATE_SYMBOL = Symbol('$state'); +export const READONLY_SYMBOL = Symbol('readonly'); const object_prototype = Object.prototype; const array_prototype = Array.prototype; diff --git a/packages/svelte/src/internal/client/proxy/readonly.js b/packages/svelte/src/internal/client/proxy/readonly.js index f0dbea76a1..e6dc2f3828 100644 --- a/packages/svelte/src/internal/client/proxy/readonly.js +++ b/packages/svelte/src/internal/client/proxy/readonly.js @@ -1,18 +1,16 @@ -import { define_property, get_descriptor } from '../utils.js'; +import { define_property } from '../utils.js'; +import { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js'; /** * @template {Record} T * @typedef {T & { [READONLY_SYMBOL]: Proxy }} StateObject */ -export const READONLY_SYMBOL = Symbol('readonly'); - -const object_prototype = Object.prototype; -const array_prototype = Array.prototype; -const get_prototype_of = Object.getPrototypeOf; const is_frozen = Object.isFrozen; /** + * Expects a value that was wrapped with `proxy` and makes it readonly. + * * @template {Record} T * @template {StateObject} U * @param {U} value @@ -26,25 +24,26 @@ export function readonly(value) { typeof value === 'object' && value != null && !is_frozen(value) && + STATE_SYMBOL in value && // TODO handle Map and Set as well !(READONLY_SYMBOL in value) ) { - const prototype = get_prototype_of(value); - - // TODO handle Map and Set as well - if (prototype === object_prototype || prototype === array_prototype) { - const proxy = new Proxy(value, handler); - define_property(value, READONLY_SYMBOL, { value: proxy, writable: false }); - - return proxy; - } + const proxy = new Proxy(value, handler); + define_property(value, READONLY_SYMBOL, { value: proxy, writable: false }); + return proxy; } return value; } -/** @returns {never} */ -const readonly_error = () => { - throw new Error(`Props cannot be mutated, unless used with \`bind:\``); +/** + * @param {any} _ + * @param {string} prop + * @returns {never} + */ +const readonly_error = (_, prop) => { + throw new Error( + `Props cannot be mutated, unless used with \`bind:\`. Use \`bind:prop-in-question={..}\` to make \`${prop}\` settable. Fallback values can never be mutated.` + ); }; /** @type {ProxyHandler>} */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index f075d2d5a3..3ebed26019 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -4,7 +4,7 @@ import { EMPTY_FUNC, run_all } from '../common.js'; import { get_descriptor, get_descriptors, is_array } from './utils.js'; import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE, PROPS_IS_RUNES } from '../../constants.js'; import { readonly } from './proxy/readonly.js'; -import { observe } from './proxy/proxy.js'; +import { observe, proxy } from './proxy/proxy.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -1426,7 +1426,7 @@ export function prop_source(props, key, flags, default_value) { call_default_value ? default_value() : default_value; if (DEV && runes) { - value = readonly(/** @type {any} */ (value)); + value = readonly(proxy(/** @type {any} */ (value))); } } diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte new file mode 100644 index 0000000000..20f744f07d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js new file mode 100644 index 0000000000..8726eead09 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js @@ -0,0 +1,22 @@ +import { test } from '../../test'; + +// Tests that readonly bails on setters/classes +export default test({ + html: ``, + + compileOptions: { + dev: true + }, + + async test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + await btn1.click(); + await btn2.click(); + assert.htmlEqual(target.innerHTML, ``); + + await btn1.click(); + await btn2.click(); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte new file mode 100644 index 0000000000..7ee12ca306 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte @@ -0,0 +1,25 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js index 65d89b5f46..62fabeac06 100644 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js @@ -14,5 +14,6 @@ export default test({ assert.htmlEqual(target.innerHTML, ``); }, - runtime_error: 'Props cannot be mutated, unless used with `bind:`' + runtime_error: + 'Props cannot be mutated, unless used with `bind:`. Use `bind:prop-in-question={..}` to make `count` settable. Fallback values can never be mutated.' }); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js index 65d89b5f46..62fabeac06 100644 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js @@ -14,5 +14,6 @@ export default test({ assert.htmlEqual(target.innerHTML, ``); }, - runtime_error: 'Props cannot be mutated, unless used with `bind:`' + runtime_error: + 'Props cannot be mutated, unless used with `bind:`. Use `bind:prop-in-question={..}` to make `count` settable. Fallback values can never be mutated.' }); From dcdd645480ab412eb563632e70801f4d61c1d787 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Dec 2023 17:14:35 +0100 Subject: [PATCH 4/7] fix: adjust children snippet default type Needs to be void so that zero args are passed to it fixes #9744 --- .changeset/tasty-numbers-perform.md | 5 +++++ packages/svelte/elements.d.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/tasty-numbers-perform.md diff --git a/.changeset/tasty-numbers-perform.md b/.changeset/tasty-numbers-perform.md new file mode 100644 index 0000000000..b5447fe325 --- /dev/null +++ b/.changeset/tasty-numbers-perform.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: adjust children snippet default type diff --git a/packages/svelte/elements.d.ts b/packages/svelte/elements.d.ts index 4c3bef74f2..a456340283 100644 --- a/packages/svelte/elements.d.ts +++ b/packages/svelte/elements.d.ts @@ -66,7 +66,7 @@ export type MessageEventHandler = EventHandler { // Implicit children prop every element has // Add this here so that libraries doing `$props()` don't need a separate interface - children?: import('svelte').Snippet; + children?: import('svelte').Snippet; // Clipboard Events 'on:copy'?: ClipboardEventHandler | undefined | null; From edc569e73bf08c2a573b12db38e185909bc1b11b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9C=D0=B8=D1=85=D0=B0=D0=B8=D0=BB=20=D0=A2=D1=83=D0=BD?= =?UTF-8?q?=D0=B8=D0=BA?= <57989636+Link-the-elf@users.noreply.github.com> Date: Wed, 6 Dec 2023 20:26:46 +0300 Subject: [PATCH 5/7] chore: refactor is_promise function (#9794) * Refactor is_promise function * Update packages/svelte/src/internal/common.js --------- Co-authored-by: Mike Co-authored-by: Rich Harris --- packages/svelte/src/internal/common.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/common.js b/packages/svelte/src/internal/common.js index 03fc1df070..1ea482a801 100644 --- a/packages/svelte/src/internal/common.js +++ b/packages/svelte/src/internal/common.js @@ -10,11 +10,7 @@ export const EMPTY_FUNC = () => {}; * @returns {value is PromiseLike} */ export function is_promise(value) { - return ( - !!value && - (typeof value === 'object' || typeof value === 'function') && - typeof (/** @type {any} */ (value).then) === 'function' - ); + return typeof value?.then === 'function'; } /** @param {Array<() => void>} arr */ From 074615d7fd200eb3625cc6c1a9cb86a713e293de Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 6 Dec 2023 18:43:47 +0100 Subject: [PATCH 6/7] fix: prevent infinite loops stemming from invalidation method (#9811) * fix: prevent infinite loops stemming from invalidation method The logic was flawed: the captured signals where always added to the previous captured no matter what, which meant a) memory leak b) that when another one runs afterwards, it will falsely contain the signals from the previous run fixes #9788 * fix lint --- .changeset/healthy-planes-vanish.md | 5 +++ .../svelte/src/internal/client/runtime.js | 18 +++++------ .../samples/select-in-each/_config.js | 32 +++++++++++++++++++ .../samples/select-in-each/main.svelte | 11 +++++++ 4 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 .changeset/healthy-planes-vanish.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/select-in-each/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/select-in-each/main.svelte diff --git a/.changeset/healthy-planes-vanish.md b/.changeset/healthy-planes-vanish.md new file mode 100644 index 0000000000..0d6ee08b92 --- /dev/null +++ b/.changeset/healthy-planes-vanish.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent infinite loops stemming from invalidation method diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 3ebed26019..1729556b13 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -863,28 +863,28 @@ export function set_sync(signal, value) { * Invokes a function and captures all signals that are read during the invocation, * then invalidates them. * @param {() => any} fn - * @returns {Set} */ export function invalidate_inner_signals(fn) { - const previous_is_signals_recorded = is_signals_recorded; - const previous_captured_signals = captured_signals; + var previous_is_signals_recorded = is_signals_recorded; + var previous_captured_signals = captured_signals; is_signals_recorded = true; captured_signals = new Set(); + var captured = captured_signals; + var signal; try { untrack(fn); } finally { is_signals_recorded = previous_is_signals_recorded; - let signal; - for (signal of captured_signals) { - previous_captured_signals.add(signal); + if (is_signals_recorded) { + for (signal of captured_signals) { + previous_captured_signals.add(signal); + } } captured_signals = previous_captured_signals; } - let signal; - for (signal of captured_signals) { + for (signal of captured) { mutate(signal, null /* doesnt matter */); } - return captured_signals; } /** diff --git a/packages/svelte/tests/runtime-legacy/samples/select-in-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/select-in-each/_config.js new file mode 100644 index 0000000000..335c46d53d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/select-in-each/_config.js @@ -0,0 +1,32 @@ +import { flushSync } from 'svelte'; +import { ok, test } from '../../test'; + +export default test({ + html: ` + + selected: a + `, + + test({ assert, target }) { + const select = target.querySelector('select'); + ok(select); + const event = new window.Event('change'); + select.value = 'b'; + select.dispatchEvent(event); + flushSync(); + + assert.htmlEqual( + target.innerHTML, + ` + + selected: b + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/select-in-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/select-in-each/main.svelte new file mode 100644 index 0000000000..a908efad18 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/select-in-each/main.svelte @@ -0,0 +1,11 @@ + + +{#each entries as entry} + + selected: {entry.selected} +{/each} From 1e4af194047307de054e785df94e9ce7273b148b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Dec 2023 14:28:15 -0500 Subject: [PATCH 7/7] 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 --- .../phases/3-transform/client/utils.js | 96 ++++++++----------- .../client/visitors/javascript-legacy.js | 32 ++++--- .../client/visitors/javascript-runes.js | 47 ++++----- .../svelte/src/internal/client/runtime.js | 11 --- packages/svelte/src/internal/index.js | 1 - 5 files changed, 80 insertions(+), 107 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 1116afc330..20cec7a7c1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -65,18 +65,20 @@ export function serialize_get_binding(node, state) { return binding.expression; } - if (binding.kind === 'prop' && binding.node.name === '$$props') { - // Special case for $$props which only exists in the old world - return node; - } + if (binding.kind === 'prop') { + if (binding.node.name === '$$props') { + // Special case for $$props which only exists in the old world + // TODO this probably shouldn't have a 'prop' binding kind + return node; + } - if ( - binding.kind === 'prop' && - !(state.analysis.immutable ? binding.reassigned : binding.mutated) && - !binding.initial && - !state.analysis.accessors - ) { - return b.call(node); + if ( + !state.analysis.accessors && + !(state.analysis.immutable ? binding.reassigned : binding.mutated) && + !binding.initial + ) { + return b.member(b.id('$$props'), node); + } } if (binding.kind === 'legacy_reactive_import') { @@ -343,67 +345,53 @@ export function serialize_hoistable_params(node, context) { } /** - * - * @param {import('#compiler').Binding} binding * @param {import('./types').ComponentClientTransformState} state * @param {string} name - * @param {import('estree').Expression | null} [default_value] + * @param {import('estree').Expression | null} [initial] * @returns */ -export function get_props_method(binding, state, name, default_value) { +export function get_prop_source(state, name, initial) { /** @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) { - let flags = 0; + let flags = 0; - /** @type {import('estree').Expression | undefined} */ - let arg; + if (state.analysis.immutable) { + flags |= PROPS_IS_IMMUTABLE; + } - if (state.analysis.immutable) { - flags |= PROPS_IS_IMMUTABLE; - } + if (state.analysis.runes) { + flags |= PROPS_IS_RUNES; + } - if (state.analysis.runes) { - flags |= PROPS_IS_RUNES; - } + /** @type {import('estree').Expression | undefined} */ + let arg; - if (default_value) { - // To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary - if (is_simple_expression(default_value)) { - arg = default_value; + if (initial) { + // To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary + if (is_simple_expression(initial)) { + arg = initial; + } else { + if ( + initial.type === 'CallExpression' && + initial.callee.type === 'Identifier' && + initial.arguments.length === 0 + ) { + arg = initial.callee; } else { - if ( - default_value.type === 'CallExpression' && - default_value.callee.type === 'Identifier' && - default_value.arguments.length === 0 - ) { - arg = default_value.callee; - } else { - arg = b.thunk(default_value); - } - - flags |= PROPS_CALL_DEFAULT_VALUE; + arg = b.thunk(initial); } - } - if (flags || arg) { - args.push(b.literal(flags)); - if (arg) args.push(arg); + flags |= PROPS_CALL_DEFAULT_VALUE; } + } - return b.call('$.prop_source', ...args); + if (flags || arg) { + args.push(b.literal(flags)); + if (arg) args.push(arg); } - return b.call('$.prop', ...args); + return b.call('$.prop_source', ...args); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js index bfc7998b86..ab2e4e3aaf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js @@ -1,7 +1,7 @@ import { is_hoistable_function } from '../../utils.js'; import * as b from '../../../../utils/builders.js'; import { extract_paths } from '../../../../utils/ast.js'; -import { create_state_declarators, get_props_method, serialize_get_binding } from '../utils.js'; +import { create_state_declarators, get_prop_source, serialize_get_binding } from '../utils.js'; /** @type {import('../types.js').ComponentVisitors} */ export const javascript_visitors_legacy = { @@ -54,8 +54,8 @@ export const javascript_visitors_legacy = { declarations.push( b.declarator( path.node, - binding.kind === 'prop' || binding.kind === 'rest_prop' - ? get_props_method(binding, state, binding.prop_alias ?? name, value) + binding.kind === 'prop' + ? get_prop_source(state, binding.prop_alias ?? name, value) : value ) ); @@ -67,17 +67,23 @@ export const javascript_visitors_legacy = { state.scope.get(declarator.id.name) ); - declarations.push( - b.declarator( - declarator.id, - get_props_method( - binding, - state, - binding.prop_alias ?? declarator.id.name, - declarator.init && /** @type {import('estree').Expression} */ (visit(declarator.init)) + if ( + state.analysis.accessors || + (state.analysis.immutable ? binding.reassigned : binding.mutated) || + declarator.init + ) { + declarations.push( + b.declarator( + declarator.id, + get_prop_source( + state, + binding.prop_alias ?? declarator.id.name, + declarator.init && + /** @type {import('estree').Expression} */ (visit(declarator.init)) + ) ) - ) - ); + ); + } continue; } 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 e84efe1582..3bc15943b5 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 @@ -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, should_proxy } from '../utils.js'; +import { create_state_declarators, get_prop_source, should_proxy } from '../utils.js'; import { unwrap_ts_expression } from '../../../../utils/ast.js'; /** @type {import('../types.js').ComponentVisitors} */ @@ -165,36 +165,27 @@ export const javascript_visitors_runes = { for (const property of declarator.id.properties) { if (property.type === 'Property') { - assert.ok(property.key.type === 'Identifier' || property.key.type === 'Literal'); - let name; - if (property.key.type === 'Identifier') { - name = property.key.name; - } else if (property.key.type === 'Literal') { - name = /** @type {string} */ (property.key.value).toString(); - } else { - throw new Error('unreachable'); - } + 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); - if (property.value.type === 'Identifier') { - const binding = /** @type {import('#compiler').Binding} */ ( - state.scope.get(property.value.name) - ); - declarations.push( - b.declarator(property.value, get_props_method(binding, state, name)) - ); - } else if (property.value.type === 'AssignmentPattern') { - assert.equal(property.value.left.type, 'Identifier'); - const binding = /** @type {import('#compiler').Binding} */ ( - state.scope.get(property.value.left.name) - ); - declarations.push( - b.declarator( - property.value.left, - get_props_method(binding, state, name, property.value.right) - ) - ); + let id = property.value; + let initial = undefined; + + if (property.value.type === 'AssignmentPattern') { + id = property.value.left; + initial = property.value.right; + } + + assert.equal(id.type, 'Identifier'); + + const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); + + if (binding.reassigned || state.analysis.accessors || initial) { + declarations.push(b.declarator(id, get_prop_source(state, name, initial))); } } else { // RestElement diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 1729556b13..1d5630d1d2 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1490,17 +1490,6 @@ export function prop_source(props, key, flags, default_value) { return /** @type {import('./types.js').Signal} */ (source_signal); } -/** - * If the prop is readonly and has no fallback value, we can use this function, else we need to use `prop_source`. - * @param {Record} props - * @param {string} key - * @returns {any} - */ -export function prop(props, key) { - // TODO skip this, and rewrite as `$$props.foo` - return () => props[key]; -} - /** * @param {boolean} immutable * @param {unknown} a diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index a94a0de180..a48a454923 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -7,7 +7,6 @@ export { source, mutable_source, derived, - prop, prop_source, user_effect, render_effect,