From cb933a62959390f9b76eccd1d63c938fde36e0a8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Dec 2023 15:58:47 +0100 Subject: [PATCH] fix: better readonly checks for proxies - 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 | 3 +- .../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(+), 22 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 000000000000..fb345c559f96 --- /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 4e6db687d6d4..940b73ea9f87 100644 --- a/packages/svelte/src/internal/client/proxy/proxy.js +++ b/packages/svelte/src/internal/client/proxy/proxy.js @@ -15,12 +15,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 }} 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 f0dbea76a194..e6dc2f38280d 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 101a189b1082..1a1b49730998 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_CALL_DEFAULT_VALUE, 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; @@ -1441,7 +1442,7 @@ export function prop_source(props_obj, 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 000000000000..20f744f07d36 --- /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 000000000000..8726eead09cc --- /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 000000000000..7ee12ca30613 --- /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 65d89b5f46c7..62fabeac0657 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 65d89b5f46c7..62fabeac0657 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.' });