From 62c9292947d0ca714626d7f8d25000ec50841d1a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 5 Dec 2023 16:02:47 -0500 Subject: [PATCH] feat: make fallback prop values readonly (#9789) * WIP * update tests * only make readonly in runes mode * remove this for now * changeset * ugh * add reassignment test * tweak message --------- Co-authored-by: Rich Harris --- .changeset/lazy-months-knock.md | 5 +++++ .../src/internal/client/proxy/readonly.js | 2 +- .../svelte/src/internal/client/runtime.js | 8 +++++++- .../class-state-derived-unowned/_config.js | 15 ++++++++++----- .../class-state-derived-unowned/log.js | 2 ++ .../class-state-derived-unowned/main.svelte | 14 +++++++------- .../class-state-init-eager-2/_config.js | 9 +++++++-- .../samples/class-state-init-eager-2/log.js | 2 ++ .../class-state-init-eager-2/main.svelte | 2 +- .../class-state-init-eager-3/_config.js | 9 +++++++-- .../samples/class-state-init-eager-3/log.js | 2 ++ .../class-state-init-eager-3/main.svelte | 2 +- .../samples/class-state-init-eager/_config.js | 9 +++++++-- .../samples/class-state-init-eager/log.js | 2 ++ .../class-state-init-eager/main.svelte | 2 +- .../event-attribute-template/_config.js | 9 +++++++-- .../samples/event-attribute-template/log.js | 2 ++ .../event-attribute-template/main.svelte | 2 +- .../Counter.svelte | 8 ++++++++ .../_config.js | 19 +++++++++++++++++++ .../main.svelte | 5 +++++ .../Counter.svelte | 8 ++++++++ .../proxy-prop-default-readonly/_config.js | 18 ++++++++++++++++++ .../proxy-prop-default-readonly/main.svelte | 5 +++++ .../samples/proxy-prop-readonly/_config.js | 2 +- 25 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 .changeset/lazy-months-knock.md create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/log.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-init-eager-2/log.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-init-eager-3/log.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-init-eager/log.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-template/log.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte diff --git a/.changeset/lazy-months-knock.md b/.changeset/lazy-months-knock.md new file mode 100644 index 0000000000..193e628fcf --- /dev/null +++ b/.changeset/lazy-months-knock.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: make fallback prop values readonly diff --git a/packages/svelte/src/internal/client/proxy/readonly.js b/packages/svelte/src/internal/client/proxy/readonly.js index fa698c3b08..f0dbea76a1 100644 --- a/packages/svelte/src/internal/client/proxy/readonly.js +++ b/packages/svelte/src/internal/client/proxy/readonly.js @@ -44,7 +44,7 @@ export function readonly(value) { /** @returns {never} */ const readonly_error = () => { - throw new Error(`Props are read-only, unless used with \`bind:\``); + throw new Error(`Props cannot be mutated, unless used with \`bind:\``); }; /** @type {ProxyHandler>} */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 08c5f78f2e..101a189b10 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -3,6 +3,7 @@ import { subscribe_to_store } from '../../store/utils.js'; 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'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -1422,13 +1423,14 @@ export function is_store(val) { export function prop_source(props_obj, key, flags, default_value) { const call_default_value = (flags & PROPS_CALL_DEFAULT_VALUE) !== 0; const immutable = (flags & PROPS_IS_IMMUTABLE) !== 0; + const runes = (flags & PROPS_IS_RUNES) !== 0; const props = is_signal(props_obj) ? get(props_obj) : props_obj; const update_bound_prop = get_descriptor(props, key)?.set; let value = props[key]; const should_set_default_value = value === undefined && default_value !== undefined; - if (update_bound_prop && default_value !== undefined && (flags & PROPS_IS_RUNES) !== 0) { + if (update_bound_prop && runes && default_value !== undefined) { // TODO consolidate all these random runtime errors throw new Error('Cannot use fallback values with bind:'); } @@ -1437,6 +1439,10 @@ export function prop_source(props_obj, key, flags, default_value) { value = // @ts-expect-error would need a cumbersome method overload to type this call_default_value ? default_value() : default_value; + + if (DEV && runes) { + value = readonly(/** @type {any} */ (value)); + } } const source_signal = immutable ? source(value) : mutable_source(value); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js index d73f3351a3..e0f3a58f3f 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/_config.js @@ -1,36 +1,41 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; +import { log } from './log.js'; export default test({ // The component context class instance gets shared between tests, strangely, causing hydration to fail? skip_if_hydrate: 'permanent', - async test({ assert, target, component }) { + before_test() { + log.length = 0; + }, + + async test({ assert, target }) { const btn = target.querySelector('button'); flushSync(() => { btn?.click(); }); - assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1]); + assert.deepEqual(log, [0, 'class trigger false', 'local trigger false', 1]); flushSync(() => { btn?.click(); }); - assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1, 2]); + assert.deepEqual(log, [0, 'class trigger false', 'local trigger false', 1, 2]); flushSync(() => { btn?.click(); }); - assert.deepEqual(component.log, [0, 'class trigger false', 'local trigger false', 1, 2, 3]); + assert.deepEqual(log, [0, 'class trigger false', 'local trigger false', 1, 2, 3]); flushSync(() => { btn?.click(); }); - assert.deepEqual(component.log, [ + assert.deepEqual(log, [ 0, 'class trigger false', 'local trigger false', diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/log.js b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/log.js new file mode 100644 index 0000000000..d3df521f4d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/log.js @@ -0,0 +1,2 @@ +/** @type {any[]} */ +export const log = []; diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte index 58b96457db..6f1c26db04 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-state-derived-unowned/main.svelte @@ -1,17 +1,17 @@ + + diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js new file mode 100644 index 0000000000..9787b1ca92 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js @@ -0,0 +1,19 @@ +import { test } from '../../test'; + +export default test({ + html: ``, + + compileOptions: { + dev: true + }, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + + await btn?.click(); + assert.htmlEqual(target.innerHTML, ``); + + await btn?.click(); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte new file mode 100644 index 0000000000..391afc4b23 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte new file mode 100644 index 0000000000..91ccc2af81 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte @@ -0,0 +1,8 @@ + + + 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 new file mode 100644 index 0000000000..65d89b5f46 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js @@ -0,0 +1,18 @@ +import { test } from '../../test'; + +export default test({ + html: ``, + + compileOptions: { + dev: true + }, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + await btn?.click(); + + assert.htmlEqual(target.innerHTML, ``); + }, + + runtime_error: 'Props cannot be mutated, unless used with `bind:`' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte new file mode 100644 index 0000000000..391afc4b23 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte @@ -0,0 +1,5 @@ + + + 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 5e0eb0e2da..65d89b5f46 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,5 @@ export default test({ assert.htmlEqual(target.innerHTML, ``); }, - runtime_error: 'Props are read-only, unless used with `bind:`' + runtime_error: 'Props cannot be mutated, unless used with `bind:`' });