Skip to content

fix: better readonly checks for proxies #9808

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 2 commits into from
Dec 6, 2023
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/five-tigers-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: better readonly checks for proxies
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/proxy/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ import {
is_array,
object_keys
} from '../utils.js';
import { READONLY_SYMBOL } from './readonly.js';

/** @typedef {{ s: Map<string | symbol, import('../types.js').SourceSignal<any>>; v: import('../types.js').SourceSignal<number>; a: boolean, i: boolean }} Metadata */
/** @typedef {Record<string | symbol, any> & { [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;
Expand Down
35 changes: 17 additions & 18 deletions packages/svelte/src/internal/client/proxy/readonly.js
Original file line number Diff line number Diff line change
@@ -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<string | symbol, any>} T
* @typedef {T & { [READONLY_SYMBOL]: Proxy<T> }} 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<string | symbol, any>} T
* @template {StateObject<T>} U
* @param {U} value
Expand All @@ -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<StateObject<any>>} */
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1415,7 +1415,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)));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
/** @type {{ object: { count: number }}} */
let { object } = $props();
</script>

<button onclick={() => object = { count: object.count + 1 } }>
clicks: {object.count}
</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { test } from '../../test';

// Tests that readonly bails on setters/classes
export default test({
html: `<button>clicks: 0</button><button>clicks: 0</button>`,

compileOptions: {
dev: true
},

async test({ assert, target }) {
const [btn1, btn2] = target.querySelectorAll('button');

await btn1.click();
await btn2.click();
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button><button>clicks: 1</button>`);

await btn1.click();
await btn2.click();
assert.htmlEqual(target.innerHTML, `<button>clicks: 2</button><button>clicks: 2</button>`);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<script>
import Counter from './Counter.svelte';

function createCounter() {
let count = $state(0)
return {
get count() {
return count;
},
set count(upd) {
count = upd
}
}
}

class CounterClass {
count = $state(0);
}

const counterSetter = createCounter();
const counterClass = new CounterClass();
</script>

<Counter object={counterSetter} />
<Counter object={counterClass} />
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ export default test({
assert.htmlEqual(target.innerHTML, `<button>clicks: 0</button>`);
},

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.'
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ export default test({
assert.htmlEqual(target.innerHTML, `<button>clicks: 0</button>`);
},

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.'
});