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/.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/.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/.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/.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; 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/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index d793a4b79c..0d8c2b07ad 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -74,19 +74,11 @@ export function serialize_get_binding(node, state) { if ( !state.analysis.accessors && - !(state.analysis.runes ? binding.reassigned : binding.mutated) && + !(state.analysis.immutable ? binding.reassigned : binding.mutated) && !binding.initial ) { return b.member(b.id('$$props'), node); } - - if ( - !(state.analysis.immutable ? binding.reassigned : binding.mutated) && - !binding.initial && - !state.analysis.accessors - ) { - return b.call(node); - } } if (binding.kind === 'legacy_reactive_import') { 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/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 b1994035ef..8787597726 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -4,6 +4,7 @@ import { EMPTY_FUNC, run_all } from '../common.js'; import { get_descriptor, get_descriptors, is_array } from './utils.js'; import { PROPS_IS_LAZY_INITIAL, PROPS_IS_IMMUTABLE, PROPS_IS_RUNES } from '../../constants.js'; import { readonly } from './proxy/readonly.js'; +import { proxy } from './proxy/proxy.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -862,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; } /** @@ -1272,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} @@ -1412,7 +1424,7 @@ export function prop_source(props, key, flags, initial) { value = (flags & PROPS_IS_LAZY_INITIAL) !== 0 ? initial() : initial; if (DEV && runes) { - value = readonly(/** @type {any} */ (value)); + value = readonly(proxy(/** @type {any} */ (value))); } } 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 */ diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 51de0ac8a2..a48a454923 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -11,6 +11,7 @@ export { user_effect, render_effect, pre_effect, + invalidate_effect, flushSync, bubble_event, safe_equal, 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} 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 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.' }); 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 @@ +[]