From f0d3740e307b8eef2332b47f2d26c4e469692ac3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Feb 2024 11:44:55 +0100 Subject: [PATCH 01/46] fix: remove readonly validation The readonly dev time validation results in false-negative object equality checks: The original object is proxified and thus comparisons could be between the unproxified and proxified version, which will always return false. Fixes #10372 There's also the problem that an object could be passed into a component but then passed upwards into shared state. At that point, it should no longer be readonly, but it's not possible to statically analyze these points. Fixes #10372 Lastly, the each block logic mutates an internal array and that also throws errors with the readonly logic. Fixes #10037 --- .changeset/tough-houses-sparkle.md | 5 ++ .../3-transform/client/visitors/template.js | 19 ++--- packages/svelte/src/internal/client/proxy.js | 80 +------------------ .../svelte/src/internal/client/runtime.js | 10 +-- .../svelte/src/internal/client/types.d.ts | 8 +- packages/svelte/src/internal/index.js | 2 +- .../samples/props-array-each/_config.js | 15 ++++ .../samples/props-array-each/child.svelte | 15 ++++ .../samples/props-array-each/main.svelte | 12 +++ .../samples/props-equality/_config.js | 45 +++++++++++ .../samples/props-equality/item.svelte | 7 ++ .../samples/props-equality/main.svelte | 19 +++++ .../Counter.svelte | 8 -- .../_config.js | 22 ----- .../main.svelte | 25 ------ .../Counter.svelte | 8 -- .../_config.js | 19 ----- .../main.svelte | 5 -- .../Counter.svelte | 8 -- .../proxy-prop-default-readonly/_config.js | 19 ----- .../proxy-prop-default-readonly/main.svelte | 5 -- .../proxy-prop-readonly/Counter.svelte | 8 -- .../samples/proxy-prop-readonly/_config.js | 19 ----- .../samples/proxy-prop-readonly/main.svelte | 7 -- 24 files changed, 131 insertions(+), 259 deletions(-) create mode 100644 .changeset/tough-houses-sparkle.md create mode 100644 packages/svelte/tests/runtime-runes/samples/props-array-each/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/props-array-each/child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/props-array-each/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/props-equality/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/props-equality/item.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/props-equality/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte diff --git a/.changeset/tough-houses-sparkle.md b/.changeset/tough-houses-sparkle.md new file mode 100644 index 000000000000..562b101834ff --- /dev/null +++ b/.changeset/tough-houses-sparkle.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: remove readonly validation as it results in false-negative object equality checks 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 5297a403cee0..b685c8349c84 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 @@ -835,8 +835,6 @@ function serialize_inline_component(node, component_name, context) { arg = b.call('$.get', id); } - if (context.state.options.dev) arg = b.call('$.readonly', arg); - push_prop(b.get(attribute.name, [b.return(arg)])); } else { push_prop(b.init(attribute.name, value)); @@ -2336,17 +2334,16 @@ export const template_visitors = { /** @type {import('estree').BlockStatement} */ (context.visit(node.fallback)) ) : b.literal(null); - const key_function = - node.key && ((each_type & EACH_ITEM_REACTIVE) !== 0 || context.state.options.dev) - ? b.arrow( - [node.context.type === 'Identifier' ? node.context : b.id('$$item'), index], - b.block( - declarations.concat( - b.return(/** @type {import('estree').Expression} */ (context.visit(node.key))) - ) + const key_function = node.key + ? b.arrow( + [node.context.type === 'Identifier' ? node.context : b.id('$$item'), index], + b.block( + declarations.concat( + b.return(/** @type {import('estree').Expression} */ (context.visit(node.key))) ) ) - : b.literal(null); + ) + : b.literal(null); if (node.index && each_node_meta.contains_group_binding) { // We needed to create a unique identifier for the index above, but we want to use the diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 07003a193dbd..0e5440cd56b5 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -18,12 +18,10 @@ import { get_prototype_of, is_array, is_frozen, - object_keys, object_prototype } from './utils.js'; export const STATE_SYMBOL = Symbol('$state'); -export const READONLY_SYMBOL = Symbol('readonly'); /** * @template T @@ -93,7 +91,7 @@ function unwrap(value, already_unwrapped = new Map()) { const descriptors = get_descriptors(value); already_unwrapped.set(value, obj); for (const key of keys) { - if (key === STATE_SYMBOL || (DEV && key === READONLY_SYMBOL)) continue; + if (key === STATE_SYMBOL) continue; if (descriptors[key].get) { define_property(obj, key, descriptors[key]); } else { @@ -175,9 +173,6 @@ const state_proxy_handler = { }, get(target, prop, receiver) { - if (DEV && prop === READONLY_SYMBOL) { - return Reflect.get(target, READONLY_SYMBOL); - } if (prop === STATE_SYMBOL) { return Reflect.get(target, STATE_SYMBOL); } @@ -224,9 +219,6 @@ const state_proxy_handler = { }, has(target, prop) { - if (DEV && prop === READONLY_SYMBOL) { - return Reflect.has(target, READONLY_SYMBOL); - } if (prop === STATE_SYMBOL) { return true; } @@ -250,10 +242,6 @@ const state_proxy_handler = { }, set(target, prop, value) { - if (DEV && prop === READONLY_SYMBOL) { - target[READONLY_SYMBOL] = value; - return true; - } const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); if (s !== undefined) set(s, proxy(value, metadata.i)); @@ -304,69 +292,3 @@ if (DEV) { throw new Error('Cannot set prototype of $state object'); }; } - -/** - * Expects a value that was wrapped with `proxy` and makes it readonly. - * - * @template {Record} T - * @template {import('./types.js').ProxyReadonlyObject | T} U - * @param {U} value - * @returns {Proxy | U} - */ -export function readonly(value) { - const proxy = value && value[READONLY_SYMBOL]; - if (proxy) { - const metadata = value[STATE_SYMBOL]; - // Check that the incoming value is the same proxy that this readonly symbol was created for: - // If someone copies over the readonly symbol to a new object (using Reflect.ownKeys) the referenced - // proxy could be stale and we should not return it. - if (metadata.p === value) return proxy; - } - - if ( - typeof value === 'object' && - value != null && - !is_frozen(value) && - STATE_SYMBOL in value && // TODO handle Map and Set as well - !(READONLY_SYMBOL in value) - ) { - const proxy = new Proxy( - value, - /** @type {ProxyHandler>} */ ( - readonly_proxy_handler - ) - ); - define_property(value, READONLY_SYMBOL, { value: proxy, writable: false }); - return proxy; - } - - return value; -} - -/** - * @param {any} _ - * @param {string} prop - * @returns {never} - */ -const readonly_error = (_, prop) => { - throw new Error( - `Non-bound props cannot be mutated — to make the \`${prop}\` settable, ensure the object it is used within is bound as a prop \`bind:={...}\`. Fallback values can never be mutated.` - ); -}; - -/** @type {ProxyHandler} */ -const readonly_proxy_handler = { - defineProperty: readonly_error, - deleteProperty: readonly_error, - set: readonly_error, - - get(target, prop, receiver) { - const value = Reflect.get(target, prop, receiver); - - if (!(prop in target)) { - return readonly(value); - } - - return value; - } -}; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index dbd7484fa14e..4e5df8fcb96f 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -17,7 +17,7 @@ import { PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../constants.js'; -import { READONLY_SYMBOL, STATE_SYMBOL, proxy, readonly, unstate } from './proxy.js'; +import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; export const SOURCE = 1; @@ -1619,10 +1619,6 @@ export function prop(props, key, flags, initial) { // @ts-expect-error would need a cumbersome method overload to type this if ((flags & PROPS_IS_LAZY_INITIAL) !== 0) initial = initial(); - if (DEV && runes) { - initial = readonly(proxy(/** @type {any} */ (initial))); - } - prop_value = /** @type {V} */ (initial); if (setter) setter(prop_value); @@ -2126,10 +2122,6 @@ export function freeze(value) { if (STATE_SYMBOL in value) { return object_freeze(unstate(value)); } - // If the value is already read-only then just use that - if (DEV && READONLY_SYMBOL in value) { - return value; - } // Otherwise freeze the object object_freeze(value); } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index bcc040138154..7c92e3e041d4 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -10,7 +10,7 @@ import { DYNAMIC_ELEMENT_BLOCK, SNIPPET_BLOCK } from './block.js'; -import type { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js'; +import type { STATE_SYMBOL } from './proxy.js'; import { DERIVED, EFFECT, RENDER_EFFECT, SOURCE, PRE_EFFECT, LAZY_PROPERTY } from './runtime.js'; // Put all internal types in this file. Once we convert to JSDoc, we can make this a d.ts file @@ -409,7 +409,7 @@ export interface ProxyMetadata> { /** Immutable: Whether to use a source or mutable source under the hood */ i: boolean; /** The associated proxy */ - p: ProxyStateObject | ProxyReadonlyObject; + p: ProxyStateObject; /** The original target this proxy was created for */ t: T; } @@ -417,7 +417,3 @@ export interface ProxyMetadata> { export type ProxyStateObject> = T & { [STATE_SYMBOL]: ProxyMetadata; }; - -export type ProxyReadonlyObject> = ProxyStateObject & { - [READONLY_SYMBOL]: ProxyMetadata; -}; diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index ae1748b4287c..f037c93353cd 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -44,7 +44,7 @@ export * from './client/each.js'; export * from './client/render.js'; export * from './client/validate.js'; export { raf } from './client/timing.js'; -export { proxy, readonly, unstate } from './client/proxy.js'; +export { proxy, unstate } from './client/proxy.js'; export { create_custom_element } from './client/custom-element.js'; export { child, diff --git a/packages/svelte/tests/runtime-runes/samples/props-array-each/_config.js b/packages/svelte/tests/runtime-runes/samples/props-array-each/_config.js new file mode 100644 index 000000000000..b3d63d3732e3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-array-each/_config.js @@ -0,0 +1,15 @@ +import { test } from '../../test'; + +export default test({ + html: `

1

1

1

`, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + + await btn?.click(); + assert.htmlEqual( + target.innerHTML, + `

1

2

1

2

1

2

` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/props-array-each/child.svelte b/packages/svelte/tests/runtime-runes/samples/props-array-each/child.svelte new file mode 100644 index 000000000000..9f5a561f522c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-array-each/child.svelte @@ -0,0 +1,15 @@ + + +{#each array as number} +

{number.v}

+{/each} + +{#each array as number (number)} +

{number.v}

+{/each} + +{#each array as number (number.v)} +

{number.v}

+{/each} diff --git a/packages/svelte/tests/runtime-runes/samples/props-array-each/main.svelte b/packages/svelte/tests/runtime-runes/samples/props-array-each/main.svelte new file mode 100644 index 000000000000..05e36da61645 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-array-each/main.svelte @@ -0,0 +1,12 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/props-equality/_config.js b/packages/svelte/tests/runtime-runes/samples/props-equality/_config.js new file mode 100644 index 000000000000..eda378947bdd --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-equality/_config.js @@ -0,0 +1,45 @@ +import { test } from '../../test'; + +export default test({ + html: ` + + + + `, + + async test({ assert, target }) { + let [btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button'); + + await btn1.click(); + assert.htmlEqual( + target.innerHTML, + ` + + + + ` + ); + + [btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button'); + await btn3.click(); + assert.htmlEqual( + target.innerHTML, + ` + + + + ` + ); + + [btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button'); + await btn5.click(); + assert.htmlEqual( + target.innerHTML, + ` + + + + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/props-equality/item.svelte b/packages/svelte/tests/runtime-runes/samples/props-equality/item.svelte new file mode 100644 index 000000000000..c40c50d9ce0c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-equality/item.svelte @@ -0,0 +1,7 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/props-equality/main.svelte b/packages/svelte/tests/runtime-runes/samples/props-equality/main.svelte new file mode 100644 index 000000000000..7c7ee0c31efb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-equality/main.svelte @@ -0,0 +1,19 @@ + + + + +{#each items as item (item)} + item.name = item.name + '+'} /> +{/each} + +{#each items as item (item.name)} + {console.log('hello'); item.name = item.name + '+'}} /> +{/each} + +{#each items as item} + item.name = item.name + '+'} /> +{/each} 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 deleted file mode 100644 index 20f744f07d36..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index 8726eead09cc..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js +++ /dev/null @@ -1,22 +0,0 @@ -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 deleted file mode 100644 index 7ee12ca30613..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte +++ /dev/null @@ -1,25 +0,0 @@ - - - - diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte deleted file mode 100644 index 1097b68f6f30..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index 9787b1ca9271..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js +++ /dev/null @@ -1,19 +0,0 @@ -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 deleted file mode 100644 index 391afc4b232e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte +++ /dev/null @@ -1,5 +0,0 @@ - - - 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 deleted file mode 100644 index 91ccc2af8188..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index d78db389d74e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js +++ /dev/null @@ -1,19 +0,0 @@ -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: - 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' -}); 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 deleted file mode 100644 index 391afc4b232e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte deleted file mode 100644 index d1be32683013..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index d78db389d74e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js +++ /dev/null @@ -1,19 +0,0 @@ -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: - 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' -}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte deleted file mode 100644 index e9617af17d5c..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte +++ /dev/null @@ -1,7 +0,0 @@ - - - From b126e6a3eb73ee871457b5c978f4fe98798c4f6e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 15:09:53 -0500 Subject: [PATCH 02/46] reinstate tests --- .../Counter.svelte | 8 ++++++ .../_config.js | 22 ++++++++++++++++ .../main.svelte | 25 +++++++++++++++++++ .../Counter.svelte | 8 ++++++ .../_config.js | 19 ++++++++++++++ .../main.svelte | 5 ++++ .../Counter.svelte | 8 ++++++ .../proxy-prop-default-readonly/_config.js | 19 ++++++++++++++ .../proxy-prop-default-readonly/main.svelte | 5 ++++ .../proxy-prop-readonly/Counter.svelte | 8 ++++++ .../samples/proxy-prop-readonly/_config.js | 19 ++++++++++++++ .../samples/proxy-prop-readonly/main.svelte | 7 ++++++ 12 files changed, 153 insertions(+) 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 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 create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte 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-reassigned/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte new file mode 100644 index 000000000000..1097b68f6f30 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte @@ -0,0 +1,8 @@ + + + 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 000000000000..9787b1ca9271 --- /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 000000000000..391afc4b232e --- /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 000000000000..91ccc2af8188 --- /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 000000000000..d78db389d74e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_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, ``); + }, + + runtime_error: + 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' +}); 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 000000000000..391afc4b232e --- /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/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte new file mode 100644 index 000000000000..d1be32683013 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte @@ -0,0 +1,8 @@ + + + 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 new file mode 100644 index 000000000000..d78db389d74e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_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, ``); + }, + + runtime_error: + 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte new file mode 100644 index 000000000000..e9617af17d5c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte @@ -0,0 +1,7 @@ + + + From f05319fb11173fcc8a57e9eba67d9c1d693c0eb8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 16:08:42 -0500 Subject: [PATCH 03/46] track ownership of state and mutations --- .../3-transform/client/transform-client.js | 5 ++ packages/svelte/src/internal/client/dev.js | 74 +++++++++++++++++++ packages/svelte/src/internal/client/proxy.js | 11 ++- .../svelte/src/internal/client/types.d.ts | 2 + packages/svelte/src/internal/index.js | 1 + 5 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/src/internal/client/dev.js diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 00a9d1fab83d..2fdc54a09ffb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -437,6 +437,11 @@ export function client_component(source, analysis, options) { } } + if (options.dev && state.options.filename) { + body.unshift(b.stmt(b.call(b.id('$.push_module'), b.literal(state.options.filename)))); + body.push(b.stmt(b.call(b.id('$.pop_module'), b.literal(state.options.filename)))); + } + return { type: 'Program', sourceType: 'module', diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev.js new file mode 100644 index 000000000000..1fcb7bb15a0c --- /dev/null +++ b/packages/svelte/src/internal/client/dev.js @@ -0,0 +1,74 @@ +/** @typedef {{ file: string, line: number, column: number }} Location */ + +/** @type {Record>} */ +const boundaries = {}; + +const chrome_pattern = /\((.+):(\d+):(\d+)\)$/; +const firefox_pattern = /@(.+):(\d+):(\d+)$/; + +export function get_stack() { + const stack = new Error().stack; + if (!stack) return null; + + const entries = []; + + for (const line of stack.split('\n').slice(1)) { + let match = chrome_pattern.exec(line) ?? firefox_pattern.exec(line); + + if (match) { + entries.push({ + file: match[1], + line: +match[2], + column: +match[3] + }); + } + } + + return entries; +} + +export function get_module() { + const stack = get_stack(); + if (!stack) return null; + + for (const entry of stack) { + if (entry) { + const modules = boundaries[entry.file]; + for (const module of modules) { + if (module.start.line < entry.line && module.end.line > entry.line) { + return module.filename; + } + } + } + } + + return null; +} + +/** + * @param {string} filename The original `path/to/Blah.svelte` filename + */ +export function push_module(filename) { + const start = get_stack()?.[1]; + + if (start) { + (boundaries[start.file] ??= []).push({ + start, + // @ts-expect-error + end: null, + filename + }); + } +} + +/** + * @param {string} filename + */ +export function pop_module(filename) { + const end = get_stack()?.[1]; + + if (end) { + // @ts-expect-error + boundaries[end.file].at(-1).end = end; + } +} diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 2d76c78a5f04..dfc76dcdef00 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -20,6 +20,7 @@ import { is_frozen, object_prototype } from './utils.js'; +import { get_module } from './dev.js'; export const STATE_SYMBOL = Symbol('$state'); @@ -52,7 +53,8 @@ export function proxy(value, immutable = true) { a: is_array(value), i: immutable, p: proxy, - t: value + t: value, + o: DEV ? get_module() : '' }), writable: true, enumerable: false @@ -249,6 +251,13 @@ const state_proxy_handler = { // @ts-ignore target[prop] = value; + if (DEV) { + const site = get_module(); + if (site !== metadata.o) { + console.error(`mutating state outside the component where it was created: ${site}`); + } + } + if (not_has) { // If we have mutated an array directly, we might need to // signal that length has also changed. Do it before updating metadata diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 8996e92e4081..823e8a749dfc 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -406,6 +406,8 @@ export interface ProxyMetadata> { p: ProxyStateObject; /** The original target this proxy was created for */ t: T; + /** The component that 'owns' this state, if any */ + o: string; } export type ProxyStateObject> = T & { diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 855fa43ced9a..49baca43de7f 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -40,6 +40,7 @@ export { freeze, init } from './client/runtime.js'; +export * from './client/dev.js'; export * from './client/each.js'; export * from './client/render.js'; export * from './client/validate.js'; From 17eae456b8c69300981a0f9e891b21d7795b5730 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 21:33:20 -0500 Subject: [PATCH 04/46] working? --- .../3-transform/client/transform-client.js | 11 +++- .../3-transform/client/visitors/template.js | 28 ++++++---- packages/svelte/src/internal/client/dev.js | 20 +++++-- packages/svelte/src/internal/client/proxy.js | 20 ++++--- .../svelte/src/internal/client/runtime.js | 56 ++++++++++++++++--- .../svelte/src/internal/client/types.d.ts | 4 ++ 6 files changed, 107 insertions(+), 32 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 2fdc54a09ffb..344e00992ace 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -439,7 +439,16 @@ export function client_component(source, analysis, options) { if (options.dev && state.options.filename) { body.unshift(b.stmt(b.call(b.id('$.push_module'), b.literal(state.options.filename)))); - body.push(b.stmt(b.call(b.id('$.pop_module'), b.literal(state.options.filename)))); + body.push( + b.stmt( + b.assignment( + '=', + b.member(b.id(analysis.name), b.id('filename')), + b.literal(state.options.filename) + ) + ) + ); + body.push(b.stmt(b.call(b.id('$.pop_module')))); } return { 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 ce9738a67b0e..637d9131bab3 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 @@ -763,6 +763,8 @@ function serialize_inline_component(node, component_name, context) { /** @type {import('estree').Identifier | import('estree').MemberExpression | null} */ let bind_this = null; + const binding_initializers = []; + /** * If this component has a slot property, it is a named slot within another component. In this case * the slot scope applies to the component itself, too, and not just its children. @@ -850,14 +852,21 @@ function serialize_inline_component(node, component_name, context) { if (attribute.name === 'this') { bind_this = attribute.expression; } else { - push_prop( - b.get(attribute.name, [ - b.return( - /** @type {import('estree').Expression} */ (context.visit(attribute.expression)) + const expression = /** @type {import('estree').Expression} */ ( + context.visit(attribute.expression) + ); + + binding_initializers.push( + b.stmt( + b.call( + b.id('$.pre_effect'), + b.thunk(b.call(b.id('$.add_owner'), expression, b.id(component_name))) ) - ]) + ) ); + push_prop(b.get(attribute.name, [b.return(expression)])); + const assignment = b.assignment('=', attribute.expression, b.id('$$value')); push_prop( b.set(attribute.name, [ @@ -997,14 +1006,11 @@ function serialize_inline_component(node, component_name, context) { ); } - /** @type {import('estree').Statement} */ - let statement = b.stmt(fn(context.state.node)); + const statements = [...snippet_declarations, ...binding_initializers]; - if (snippet_declarations.length > 0) { - statement = b.block([...snippet_declarations, statement]); - } + statements.push(b.stmt(fn(context.state.node))); - return statement; + return statements.length > 1 ? b.block(statements) : statements[0]; } /** diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev.js index 1fcb7bb15a0c..ca18ad6becd4 100644 --- a/packages/svelte/src/internal/client/dev.js +++ b/packages/svelte/src/internal/client/dev.js @@ -1,5 +1,7 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ +import { deep_read, set_current_owner, set_current_owner_override, untrack } from './runtime.js'; + /** @type {Record>} */ const boundaries = {}; @@ -61,10 +63,7 @@ export function push_module(filename) { } } -/** - * @param {string} filename - */ -export function pop_module(filename) { +export function pop_module() { const end = get_stack()?.[1]; if (end) { @@ -72,3 +71,16 @@ export function pop_module(filename) { boundaries[end.file].at(-1).end = end; } } + +/** + * + * @param {any} object + * @param {any} owner + */ +export function add_owner(object, owner) { + untrack(() => { + set_current_owner_override(owner.filename); + deep_read(object); + set_current_owner_override(null); + }); +} diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index dfc76dcdef00..3e53d51272db 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -8,7 +8,10 @@ import { updating_derived, UNINITIALIZED, mutable_source, - batch_inspect + batch_inspect, + set_current_owner, + current_component_context, + current_owner } from './runtime.js'; import { array_prototype, @@ -54,7 +57,7 @@ export function proxy(value, immutable = true) { i: immutable, p: proxy, t: value, - o: DEV ? get_module() : '' + o: DEV && (current_owner ?? current_component_context?.f) }), writable: true, enumerable: false @@ -178,12 +181,18 @@ const state_proxy_handler = { (effect_active() || updating_derived) && (!(prop in target) || get_descriptor(target, prop)?.writable) ) { + console.log('setting current owner', metadata.o); + const previous_owner = current_owner; + if (!current_owner) set_current_owner(metadata.o); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); + set_current_owner(previous_owner); metadata.s.set(prop, s); } if (s !== undefined) { + // set_current_owner(metadata.o); const value = get(s); + // set_current_owner(null); return value === UNINITIALIZED ? undefined : value; } @@ -251,13 +260,6 @@ const state_proxy_handler = { // @ts-ignore target[prop] = value; - if (DEV) { - const site = get_module(); - if (site !== metadata.o) { - console.error(`mutating state outside the component where it was created: ${site}`); - } - } - if (not_has) { // If we have mutated an array directly, we might need to // signal that length has also changed. Do it before updating metadata diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index f21f54214c82..72f768ef3109 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,6 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; +import { get_module } from './dev.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -106,6 +107,26 @@ export let current_component_context = null; export let updating_derived = false; +/** @type {string | null} */ +export let current_owner = null; + +/** + * @param {string | null} owner + */ +export function set_current_owner(owner) { + current_owner = owner; +} + +/** @type {string | null} */ +export let current_owner_override = null; + +/** + * @param {string | null} owner + */ +export function set_current_owner_override(owner) { + current_owner_override = owner; +} + /** * @param {null | import('./types.js').ComponentContext} context * @returns {boolean} @@ -913,11 +934,17 @@ export function unsubscribe_on_destroy(stores) { * @returns {V} */ export function get(signal) { - // @ts-expect-error - if (DEV && signal.inspect && inspect_fn) { - /** @type {import('./types.js').SignalDebug} */ (signal).inspect.add(inspect_fn); + if (DEV) { + if (signal.o && (current_owner_override || current_owner)) { + signal.o.add(current_owner_override || current_owner); + } + // @ts-expect-error - inspect_captured_signals.push(signal); + if (signal.inspect && inspect_fn) { + /** @type {import('./types.js').SignalDebug} */ (signal).inspect.add(inspect_fn); + // @ts-expect-error + inspect_captured_signals.push(signal); + } } const flags = signal.f; @@ -983,6 +1010,13 @@ export function get(signal) { * @returns {V} */ export function set(signal, value) { + if (DEV && 'o' in signal) { + const site = get_module(); + if (site && !signal.o.has(site)) { + console.error(`mutating state outside the component where it was created: ${site}`); + } + } + set_signal_value(signal, value); return value; } @@ -1295,6 +1329,13 @@ export function source(initial_value) { * @param {import('./types.js').Signal} signal */ function bind_signal_to_component_context(signal) { + if (DEV) { + const owner = current_owner ?? current_component_context?.f; + if (owner) { + (signal.o ??= new Set()).add(owner); + } + } + if (current_component_context === null || !current_component_context.r) return; const signals = current_component_context.d; @@ -1858,7 +1899,6 @@ function on_destroy(fn) { */ export function push(props, runes = false) { current_component_context = { - // accessors a: null, // context c: null, @@ -1875,7 +1915,9 @@ export function push(props, runes = false) { // runes r: runes, // update_callbacks - u: null + u: null, + // filename + f: (DEV && get_module()) || '' }; } @@ -1956,7 +1998,7 @@ export function init() { * @param {Set} visited * @returns {void} */ -function deep_read(value, visited = new Set()) { +export function deep_read(value, visited = new Set()) { if (typeof value === 'object' && value !== null && !visited.has(value)) { visited.add(value); for (let key in value) { diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 823e8a749dfc..fb1ef50330f0 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -61,6 +61,8 @@ export type ComponentContext = { /** onMount callbacks */ m: Array<() => any>; }; + /** filename */ + f: string; }; // We keep two shapes rather than a single monomorphic shape to improve the memory usage. @@ -77,6 +79,8 @@ export type SourceSignal = { f: SignalFlags; /** value: The latest value for this signal */ v: V; + /** owners: The components that can write to this signal */ + o: Set; }; export type SourceSignalDebug = { From 1e48ccbcc71653ba6d113bf3b1f9fdefa68d5568 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 21:35:06 -0500 Subject: [PATCH 05/46] remove old changeset --- .changeset/tough-houses-sparkle.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/tough-houses-sparkle.md diff --git a/.changeset/tough-houses-sparkle.md b/.changeset/tough-houses-sparkle.md deleted file mode 100644 index 562b101834ff..000000000000 --- a/.changeset/tough-houses-sparkle.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"svelte": patch ---- - -fix: remove readonly validation as it results in false-negative object equality checks From 446e15c611d038ae0916e5ba16946e017a750025 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 21:37:00 -0500 Subject: [PATCH 06/46] tidy --- packages/svelte/src/internal/client/proxy.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 3e53d51272db..ace642077ec6 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -181,7 +181,6 @@ const state_proxy_handler = { (effect_active() || updating_derived) && (!(prop in target) || get_descriptor(target, prop)?.writable) ) { - console.log('setting current owner', metadata.o); const previous_owner = current_owner; if (!current_owner) set_current_owner(metadata.o); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); @@ -190,9 +189,7 @@ const state_proxy_handler = { } if (s !== undefined) { - // set_current_owner(metadata.o); const value = get(s); - // set_current_owner(null); return value === UNINITIALIZED ? undefined : value; } From 918faacf3e490d7ca27474e815aa2af22431d413 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 22:20:36 -0500 Subject: [PATCH 07/46] error --- packages/svelte/src/internal/client/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 72f768ef3109..2677de2a345c 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1013,7 +1013,7 @@ export function set(signal, value) { if (DEV && 'o' in signal) { const site = get_module(); if (site && !signal.o.has(site)) { - console.error(`mutating state outside the component where it was created: ${site}`); + throw new Error(`mutating state outside the component where it was created: ${site}`); } } From a6b151d627c08965db7d3a1fd82b8065ce840a68 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 22:34:09 -0500 Subject: [PATCH 08/46] simplify --- .../3-transform/client/transform-client.js | 11 +---------- packages/svelte/src/internal/client/dev.js | 16 ++++++++-------- packages/svelte/src/internal/client/proxy.js | 2 +- packages/svelte/src/internal/client/runtime.js | 16 ++++++++-------- packages/svelte/src/internal/client/types.d.ts | 6 +++--- 5 files changed, 21 insertions(+), 30 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 344e00992ace..167d4efc3a10 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -438,16 +438,7 @@ export function client_component(source, analysis, options) { } if (options.dev && state.options.filename) { - body.unshift(b.stmt(b.call(b.id('$.push_module'), b.literal(state.options.filename)))); - body.push( - b.stmt( - b.assignment( - '=', - b.member(b.id(analysis.name), b.id('filename')), - b.literal(state.options.filename) - ) - ) - ); + body.unshift(b.stmt(b.call(b.id('$.push_module'), b.id(analysis.name)))); body.push(b.stmt(b.call(b.id('$.pop_module')))); } diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev.js index ca18ad6becd4..fc4e24695266 100644 --- a/packages/svelte/src/internal/client/dev.js +++ b/packages/svelte/src/internal/client/dev.js @@ -1,8 +1,8 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { deep_read, set_current_owner, set_current_owner_override, untrack } from './runtime.js'; +import { deep_read, set_current_owner_override, untrack } from './runtime.js'; -/** @type {Record>} */ +/** @type {Record>} */ const boundaries = {}; const chrome_pattern = /\((.+):(\d+):(\d+)\)$/; @@ -29,7 +29,7 @@ export function get_stack() { return entries; } -export function get_module() { +export function get_component() { const stack = get_stack(); if (!stack) return null; @@ -38,7 +38,7 @@ export function get_module() { const modules = boundaries[entry.file]; for (const module of modules) { if (module.start.line < entry.line && module.end.line > entry.line) { - return module.filename; + return module.component; } } } @@ -48,9 +48,9 @@ export function get_module() { } /** - * @param {string} filename The original `path/to/Blah.svelte` filename + * @param {Function} component */ -export function push_module(filename) { +export function push_module(component) { const start = get_stack()?.[1]; if (start) { @@ -58,7 +58,7 @@ export function push_module(filename) { start, // @ts-expect-error end: null, - filename + component }); } } @@ -79,7 +79,7 @@ export function pop_module() { */ export function add_owner(object, owner) { untrack(() => { - set_current_owner_override(owner.filename); + set_current_owner_override(owner); deep_read(object); set_current_owner_override(null); }); diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index ace642077ec6..042495ef370a 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -23,7 +23,7 @@ import { is_frozen, object_prototype } from './utils.js'; -import { get_module } from './dev.js'; +import { get_component } from './dev.js'; export const STATE_SYMBOL = Symbol('$state'); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2677de2a345c..b527e58235b6 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { get_module } from './dev.js'; +import { get_component } from './dev.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -117,11 +117,11 @@ export function set_current_owner(owner) { current_owner = owner; } -/** @type {string | null} */ +/** @type {Function | null} */ export let current_owner_override = null; /** - * @param {string | null} owner + * @param {Function | null} owner */ export function set_current_owner_override(owner) { current_owner_override = owner; @@ -1011,9 +1011,9 @@ export function get(signal) { */ export function set(signal, value) { if (DEV && 'o' in signal) { - const site = get_module(); - if (site && !signal.o.has(site)) { - throw new Error(`mutating state outside the component where it was created: ${site}`); + const component = get_component(); + if (component && !signal.o.has(component)) { + throw new Error(`illegal mutation`); // TODO better error } } @@ -1916,8 +1916,8 @@ export function push(props, runes = false) { r: runes, // update_callbacks u: null, - // filename - f: (DEV && get_module()) || '' + // component function (TODO maybe just pass this in, in dev?) + f: DEV && get_component() }; } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index fb1ef50330f0..bbe66b108220 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -61,8 +61,8 @@ export type ComponentContext = { /** onMount callbacks */ m: Array<() => any>; }; - /** filename */ - f: string; + /** component function */ + f: null | Function; }; // We keep two shapes rather than a single monomorphic shape to improve the memory usage. @@ -80,7 +80,7 @@ export type SourceSignal = { /** value: The latest value for this signal */ v: V; /** owners: The components that can write to this signal */ - o: Set; + o: Set; }; export type SourceSignalDebug = { From 5213c72b14989e464f5ba957aa985e82fa7847d1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 22:37:21 -0500 Subject: [PATCH 09/46] fix --- packages/svelte/src/internal/client/runtime.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index b527e58235b6..5f2e99e6566f 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -935,8 +935,8 @@ export function unsubscribe_on_destroy(stores) { */ export function get(signal) { if (DEV) { - if (signal.o && (current_owner_override || current_owner)) { - signal.o.add(current_owner_override || current_owner); + if (signal.o && current_owner_override) { + signal.o.add(current_owner_override); } // @ts-expect-error From 7e48b13044dc71e8c51abe49cfec35756971223f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 22:43:16 -0500 Subject: [PATCH 10/46] fix --- packages/svelte/src/internal/client/dev.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev.js index fc4e24695266..2780b20c3017 100644 --- a/packages/svelte/src/internal/client/dev.js +++ b/packages/svelte/src/internal/client/dev.js @@ -34,12 +34,12 @@ export function get_component() { if (!stack) return null; for (const entry of stack) { - if (entry) { - const modules = boundaries[entry.file]; - for (const module of modules) { - if (module.start.line < entry.line && module.end.line > entry.line) { - return module.component; - } + const modules = boundaries[entry.file]; + if (!modules) continue; + + for (const module of modules) { + if (module.start.line < entry.line && module.end.line > entry.line) { + return module.component; } } } From 26c5fbbdaf84fb2da3186b9f375871d73119147d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 23:05:03 -0500 Subject: [PATCH 11/46] fix --- packages/svelte/src/internal/client/dev.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev.js index 2780b20c3017..68933d32ece7 100644 --- a/packages/svelte/src/internal/client/dev.js +++ b/packages/svelte/src/internal/client/dev.js @@ -5,7 +5,7 @@ import { deep_read, set_current_owner_override, untrack } from './runtime.js'; /** @type {Record>} */ const boundaries = {}; -const chrome_pattern = /\((.+):(\d+):(\d+)\)$/; +const chrome_pattern = /at (?:.+ \()?(.+):(\d+):(\d+)\)?$/; const firefox_pattern = /@(.+):(\d+):(\d+)$/; export function get_stack() { @@ -14,7 +14,7 @@ export function get_stack() { const entries = []; - for (const line of stack.split('\n').slice(1)) { + for (const line of stack.split('\n')) { let match = chrome_pattern.exec(line) ?? firefox_pattern.exec(line); if (match) { @@ -51,7 +51,7 @@ export function get_component() { * @param {Function} component */ export function push_module(component) { - const start = get_stack()?.[1]; + const start = get_stack()?.[2]; if (start) { (boundaries[start.file] ??= []).push({ @@ -64,7 +64,7 @@ export function push_module(component) { } export function pop_module() { - const end = get_stack()?.[1]; + const end = get_stack()?.[2]; if (end) { // @ts-expect-error From 07812119b91d7239a8b64f43d2ba3cab8b1076ac Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 12 Feb 2024 23:08:21 -0500 Subject: [PATCH 12/46] tidy --- .../phases/3-transform/client/visitors/template.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 637d9131bab3..237acab54528 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 @@ -1006,9 +1006,11 @@ function serialize_inline_component(node, component_name, context) { ); } - const statements = [...snippet_declarations, ...binding_initializers]; - - statements.push(b.stmt(fn(context.state.node))); + const statements = [ + ...snippet_declarations, + ...binding_initializers, + b.stmt(fn(context.state.node)) + ]; return statements.length > 1 ? b.block(statements) : statements[0]; } From 00b6626feffecbfb54f0881313bed57ef08ec58d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 12:37:54 -0500 Subject: [PATCH 13/46] make it a warning --- .../svelte/src/internal/client/runtime.js | 5 ++- .../samples/proxy-prop-readonly/_config.js | 36 ++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 926a35c9592f..4975d9da32f4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1034,7 +1034,10 @@ export function set(signal, value) { if (DEV && 'o' in signal) { const component = get_component(); if (component && !signal.o.has(component)) { - throw new Error(`illegal mutation`); // TODO better error + console.warn( + 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' + ); + console.trace(); } } 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 d78db389d74e..81b3782158f9 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 @@ -1,5 +1,14 @@ import { test } from '../../test'; +/** @type {typeof console.warn} */ +let warn; + +/** @type {typeof console.trace} */ +let trace; + +/** @type {any[]} */ +let warnings = []; + export default test({ html: ``, @@ -7,13 +16,32 @@ export default test({ dev: true }, + before_test: () => { + warn = console.warn; + trace = console.trace; + + console.warn = (...args) => { + warnings.push(...args); + }; + + console.trace = () => {}; + }, + + after_test: () => { + console.warn = warn; + console.trace = trace; + + warnings = []; + }, + async test({ assert, target }) { const btn = target.querySelector('button'); await btn?.click(); - assert.htmlEqual(target.innerHTML, ``); - }, + assert.htmlEqual(target.innerHTML, ``); - runtime_error: - 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' + assert.deepEqual(warnings, [ + 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' + ]); + } }); From 7b3fede1909f78ed3cec426925815297a20beaca Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 12:47:13 -0500 Subject: [PATCH 14/46] rename test --- .../Counter.svelte | 0 .../_config.js | 0 .../main.svelte | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename packages/svelte/tests/runtime-runes/samples/{proxy-prop-readonly => non-local-mutation-discouraged}/Counter.svelte (100%) rename packages/svelte/tests/runtime-runes/samples/{proxy-prop-readonly => non-local-mutation-discouraged}/_config.js (100%) rename packages/svelte/tests/runtime-runes/samples/{proxy-prop-readonly => non-local-mutation-discouraged}/main.svelte (100%) diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/Counter.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/Counter.svelte rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/Counter.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/main.svelte rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/main.svelte From fc2dec13d304d44551ed162629241658391d6360 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 12:49:10 -0500 Subject: [PATCH 15/46] remove unused test --- .../Counter.svelte | 8 ------ .../_config.js | 22 ---------------- .../main.svelte | 25 ------------------- 3 files changed, 55 deletions(-) delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte 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 deleted file mode 100644 index 20f744f07d36..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index 8726eead09cc..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js +++ /dev/null @@ -1,22 +0,0 @@ -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 deleted file mode 100644 index 7ee12ca30613..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte +++ /dev/null @@ -1,25 +0,0 @@ - - - - From 8cd72c032beb48385181f23debba53a13a5b15ac Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 12:53:17 -0500 Subject: [PATCH 16/46] update tests --- .../Counter.svelte | 6 +++- .../props-default-reactivity/_config.js | 30 +++++++++++++++++++ .../main.svelte | 0 .../_config.js | 19 ------------ .../Counter.svelte | 8 ----- .../proxy-prop-default-readonly/_config.js | 19 ------------ .../proxy-prop-default-readonly/main.svelte | 5 ---- 7 files changed, 35 insertions(+), 52 deletions(-) rename packages/svelte/tests/runtime-runes/samples/{proxy-prop-default-readonly-reassigned => props-default-reactivity}/Counter.svelte (63%) create mode 100644 packages/svelte/tests/runtime-runes/samples/props-default-reactivity/_config.js rename packages/svelte/tests/runtime-runes/samples/{proxy-prop-default-readonly-reassigned => props-default-reactivity}/main.svelte (100%) delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/props-default-reactivity/Counter.svelte similarity index 63% rename from packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte rename to packages/svelte/tests/runtime-runes/samples/props-default-reactivity/Counter.svelte index 1097b68f6f30..2ba2ed91006b 100644 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/Counter.svelte +++ b/packages/svelte/tests/runtime-runes/samples/props-default-reactivity/Counter.svelte @@ -3,6 +3,10 @@ let { object = { count: 0 } } = $props(); + + diff --git a/packages/svelte/tests/runtime-runes/samples/props-default-reactivity/_config.js b/packages/svelte/tests/runtime-runes/samples/props-default-reactivity/_config.js new file mode 100644 index 000000000000..cf703fcea591 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-default-reactivity/_config.js @@ -0,0 +1,30 @@ +import { test } from '../../test'; + +export default test({ + html: ` + + + `, + + async test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + await btn1?.click(); + assert.htmlEqual( + target.innerHTML, + ` + + + ` + ); + + await btn2?.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/props-default-reactivity/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/main.svelte rename to packages/svelte/tests/runtime-runes/samples/props-default-reactivity/main.svelte 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 deleted file mode 100644 index 9787b1ca9271..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-reassigned/_config.js +++ /dev/null @@ -1,19 +0,0 @@ -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/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte deleted file mode 100644 index 91ccc2af8188..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/Counter.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - 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 deleted file mode 100644 index d78db389d74e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js +++ /dev/null @@ -1,19 +0,0 @@ -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: - 'Non-bound props cannot be mutated — to make the `count` settable, ensure the object it is used within is bound as a prop `bind:={...}`. Fallback values can never be mutated.' -}); 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 deleted file mode 100644 index 391afc4b232e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/main.svelte +++ /dev/null @@ -1,5 +0,0 @@ - - - From 50188c30af9cf77a0bf341b123f545f967175153 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 13:02:08 -0500 Subject: [PATCH 17/46] slap ts-expect-error everywhere, because its too finicky otherwise --- packages/svelte/src/internal/client/proxy.js | 3 +- .../svelte/src/internal/client/runtime.js | 30 +++++++++++++------ .../svelte/src/internal/client/types.d.ts | 6 ++-- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 042495ef370a..7d8010917d3c 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -57,7 +57,8 @@ export function proxy(value, immutable = true) { i: immutable, p: proxy, t: value, - o: DEV && (current_owner ?? current_component_context?.f) + // @ts-expect-error + o: DEV && (current_owner ?? current_component_context?.function) }), writable: true, enumerable: false diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 4975d9da32f4..31dcb6b53ed2 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -189,7 +189,8 @@ function create_source_signal(flags, value) { // value v: value, // this is for DEV only - inspect: new Set() + inspect: new Set(), + owners: new Set() }; } return { @@ -956,8 +957,10 @@ export function unsubscribe_on_destroy(stores) { */ export function get(signal) { if (DEV) { - if (signal.o && current_owner_override) { - signal.o.add(current_owner_override); + // @ts-expect-error + if (signal.owners && current_owner_override) { + // @ts-expect-error + signal.owners.add(current_owner_override); } // @ts-expect-error @@ -1033,7 +1036,9 @@ export function get(signal) { export function set(signal, value) { if (DEV && 'o' in signal) { const component = get_component(); - if (component && !signal.o.has(component)) { + + // @ts-expect-error + if (component && !signal.owners.has(component)) { console.warn( 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' ); @@ -1354,9 +1359,12 @@ export function source(initial_value) { */ function bind_signal_to_component_context(signal) { if (DEV) { - const owner = current_owner ?? current_component_context?.f; + // @ts-expect-error + const owner = current_owner ?? current_component_context?.function; + if (owner) { - (signal.o ??= new Set()).add(owner); + // @ts-expect-error + (signal.owners ??= new Set()).add(owner); } } @@ -1940,10 +1948,14 @@ export function push(props, runes = false) { // runes r: runes, // update_callbacks - u: null, - // component function (TODO maybe just pass this in, in dev?) - f: DEV && get_component() + u: null }; + + if (DEV) { + // component function (TODO maybe just pass this in, in dev?) + // @ts-expect-error + current_component_context.function = get_component(); + } } /** diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index bbe66b108220..b23735453262 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -61,8 +61,6 @@ export type ComponentContext = { /** onMount callbacks */ m: Array<() => any>; }; - /** component function */ - f: null | Function; }; // We keep two shapes rather than a single monomorphic shape to improve the memory usage. @@ -79,13 +77,13 @@ export type SourceSignal = { f: SignalFlags; /** value: The latest value for this signal */ v: V; - /** owners: The components that can write to this signal */ - o: Set; }; export type SourceSignalDebug = { /** This is DEV only */ inspect: Set; + /** This is DEV only */ + owners: Set; }; export type ComputationSignal = { From 2f202edf9f10f93d36b8273550cbf42ecb02382d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 13:14:19 -0500 Subject: [PATCH 18/46] oops --- packages/svelte/src/internal/client/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 31dcb6b53ed2..1a35553287a6 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1034,7 +1034,7 @@ export function get(signal) { * @returns {V} */ export function set(signal, value) { - if (DEV && 'o' in signal) { + if (DEV && 'owners' in signal) { const component = get_component(); // @ts-expect-error From 49305351385f10afa7c99cfbf37dbc4a56423429 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 13:15:17 -0500 Subject: [PATCH 19/46] oh no the hall monitor is here --- packages/svelte/src/internal/client/runtime.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 1a35553287a6..7885256ba05e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1039,9 +1039,12 @@ export function set(signal, value) { // @ts-expect-error if (component && !signal.owners.has(component)) { + // eslint-disable-next-line no-console console.warn( 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' ); + + // eslint-disable-next-line no-console console.trace(); } } From 87a2514846c73a9698efd8c53563b1ca82733cd9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 13:25:09 -0500 Subject: [PATCH 20/46] only call add_owner in dev --- .../3-transform/client/visitors/template.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) 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 80276abad9f0..52dae163d050 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 @@ -857,14 +857,16 @@ function serialize_inline_component(node, component_name, context) { context.visit(attribute.expression) ); - binding_initializers.push( - b.stmt( - b.call( - b.id('$.pre_effect'), - b.thunk(b.call(b.id('$.add_owner'), expression, b.id(component_name))) + if (context.state.options.dev) { + binding_initializers.push( + b.stmt( + b.call( + b.id('$.pre_effect'), + b.thunk(b.call(b.id('$.add_owner'), expression, b.id(component_name))) + ) ) - ) - ); + ); + } push_prop(b.get(attribute.name, [b.return(expression)])); From 6f81a15375b71b3de2d6bfdc05816e901d54ea93 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 14:45:57 -0500 Subject: [PATCH 21/46] only owners can transfer ownership --- packages/svelte/src/internal/client/runtime.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 7885256ba05e..694334789207 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -957,8 +957,11 @@ export function unsubscribe_on_destroy(stores) { */ export function get(signal) { if (DEV) { - // @ts-expect-error - if (signal.owners && current_owner_override) { + if ( + current_owner_override && + // @ts-expect-error + signal.owners?.has(current_component_context.function) + ) { // @ts-expect-error signal.owners.add(current_owner_override); } From f807b7c1eb3687db8b69e43fb7857c94f86f2b00 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 14:50:04 -0500 Subject: [PATCH 22/46] simplify --- .../compiler/phases/3-transform/client/transform-client.js | 5 ++++- packages/svelte/src/internal/client/runtime.js | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 4897d7712c86..a61c0df3bb0b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -260,8 +260,11 @@ export function client_component(source, analysis, options) { } } + const push_args = [b.id('$$props'), b.literal(analysis.runes)]; + if (options.dev) push_args.push(b.id(analysis.name)); + const component_block = b.block([ - b.stmt(b.call('$.push', b.id('$$props'), b.literal(analysis.runes))), + b.stmt(b.call('$.push', ...push_args)), ...store_setup, ...legacy_reactive_declarations, ...group_binding_declarations, diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 694334789207..cd2dac4101bf 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1934,9 +1934,10 @@ function on_destroy(fn) { /** * @param {Record} props * @param {any} runes + * @param {Function} [fn] * @returns {void} */ -export function push(props, runes = false) { +export function push(props, runes = false, fn) { current_component_context = { a: null, // context @@ -1958,9 +1959,9 @@ export function push(props, runes = false) { }; if (DEV) { - // component function (TODO maybe just pass this in, in dev?) + // component function // @ts-expect-error - current_component_context.function = get_component(); + current_component_context.function = fn; } } From d665569155a04e51bbd6e6768ed52d30f3d6332e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 16:46:40 -0500 Subject: [PATCH 23/46] fixes --- packages/svelte/src/internal/client/proxy.js | 5 +-- .../svelte/src/internal/client/runtime.js | 18 +++++---- .../Counter.svelte | 8 ++++ .../_config.js | 37 +++++++++++++++++++ .../main.svelte | 7 ++++ 5 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/main.svelte diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 7d8010917d3c..3677185f46fd 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -57,8 +57,7 @@ export function proxy(value, immutable = true) { i: immutable, p: proxy, t: value, - // @ts-expect-error - o: DEV && (current_owner ?? current_component_context?.function) + o: DEV && current_owner }), writable: true, enumerable: false @@ -183,7 +182,7 @@ const state_proxy_handler = { (!(prop in target) || get_descriptor(target, prop)?.writable) ) { const previous_owner = current_owner; - if (!current_owner) set_current_owner(metadata.o); + set_current_owner(metadata.o); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); set_current_owner(previous_owner); metadata.s.set(prop, s); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index cd2dac4101bf..7fba823f0795 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -111,11 +111,11 @@ export let current_component_context = null; export let updating_derived = false; -/** @type {string | null} */ +/** @type {Function | null} */ export let current_owner = null; /** - * @param {string | null} owner + * @param {Function | null} owner */ export function set_current_owner(owner) { current_owner = owner; @@ -1041,7 +1041,7 @@ export function set(signal, value) { const component = get_component(); // @ts-expect-error - if (component && !signal.owners.has(component)) { + if (component && signal.owners.size > 0 && !signal.owners.has(component)) { // eslint-disable-next-line no-console console.warn( 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' @@ -1366,11 +1366,9 @@ export function source(initial_value) { function bind_signal_to_component_context(signal) { if (DEV) { // @ts-expect-error - const owner = current_owner ?? current_component_context?.function; - - if (owner) { + if (current_owner && signal.owners) { // @ts-expect-error - (signal.owners ??= new Set()).add(owner); + signal.owners.add(current_owner); } } @@ -1961,7 +1959,7 @@ export function push(props, runes = false, fn) { if (DEV) { // component function // @ts-expect-error - current_component_context.function = fn; + current_owner = current_component_context.function = fn; } } @@ -1983,6 +1981,10 @@ export function pop(accessors) { } } current_component_context = context_stack_item.p; + if (DEV) { + // @ts-expect-error + current_owner = current_component_context?.function; + } context_stack_item.m = true; } } diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/Counter.svelte new file mode 100644 index 000000000000..d1be32683013 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/Counter.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/_config.js new file mode 100644 index 000000000000..dc600e1461b5 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/_config.js @@ -0,0 +1,37 @@ +import { test } from '../../test'; + +/** @type {typeof console.warn} */ +let warn; + +/** @type {any[]} */ +let warnings = []; + +export default test({ + html: ``, + + compileOptions: { + dev: true + }, + + before_test: () => { + warn = console.warn; + + console.warn = (...args) => { + warnings.push(...args); + }; + }, + + after_test: () => { + console.warn = warn; + warnings = []; + }, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + await btn?.click(); + + assert.htmlEqual(target.innerHTML, ``); + + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/main.svelte new file mode 100644 index 000000000000..e8bd966fbd1b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding/main.svelte @@ -0,0 +1,7 @@ + + + From 2c0215f7a36707e93e4a428a6111a1df924f0cfc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 16:51:31 -0500 Subject: [PATCH 24/46] tidy up --- packages/svelte/src/internal/client/proxy.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 3677185f46fd..d1f2ea4faee2 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -10,7 +10,6 @@ import { mutable_source, batch_inspect, set_current_owner, - current_component_context, current_owner } from './runtime.js'; import { @@ -23,7 +22,6 @@ import { is_frozen, object_prototype } from './utils.js'; -import { get_component } from './dev.js'; export const STATE_SYMBOL = Symbol('$state'); @@ -182,9 +180,9 @@ const state_proxy_handler = { (!(prop in target) || get_descriptor(target, prop)?.writable) ) { const previous_owner = current_owner; - set_current_owner(metadata.o); + if (DEV) set_current_owner(metadata.o); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); - set_current_owner(previous_owner); + if (DEV) set_current_owner(previous_owner); metadata.s.set(prop, s); } From d4022b6cf95196df5b4ca2bc2d65fc1937d050a9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 17:20:15 -0500 Subject: [PATCH 25/46] fix type error --- packages/svelte/src/internal/client/proxy.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index d1f2ea4faee2..0c2b3f16e1e5 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -54,13 +54,17 @@ export function proxy(value, immutable = true) { a: is_array(value), i: immutable, p: proxy, - t: value, - o: DEV && current_owner + t: value }), writable: true, enumerable: false }); + if (DEV) { + // @ts-expect-error + value[STATE_SYMBOL].o = current_owner; + } + return proxy; } } From 8603e104eda0049ac92c62d42a81c38a03701f9f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 18 Feb 2024 17:20:56 -0500 Subject: [PATCH 26/46] while we're at it --- packages/svelte/src/internal/client/proxy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 0c2b3f16e1e5..0039edd5a2d4 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -62,7 +62,7 @@ export function proxy(value, immutable = true) { if (DEV) { // @ts-expect-error - value[STATE_SYMBOL].o = current_owner; + value[STATE_SYMBOL].owner = current_owner; } return proxy; @@ -184,7 +184,7 @@ const state_proxy_handler = { (!(prop in target) || get_descriptor(target, prop)?.writable) ) { const previous_owner = current_owner; - if (DEV) set_current_owner(metadata.o); + if (DEV) set_current_owner(metadata.owner); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); if (DEV) set_current_owner(previous_owner); metadata.s.set(prop, s); From 7b96a7d25b0112194f8df888807bf8a8b6fbc85c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 16:58:28 -0500 Subject: [PATCH 27/46] rename file --- .../svelte/src/internal/client/{dev.js => dev/ownership.js} | 2 +- packages/svelte/src/internal/client/runtime.js | 2 +- packages/svelte/src/internal/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename packages/svelte/src/internal/client/{dev.js => dev/ownership.js} (99%) diff --git a/packages/svelte/src/internal/client/dev.js b/packages/svelte/src/internal/client/dev/ownership.js similarity index 99% rename from packages/svelte/src/internal/client/dev.js rename to packages/svelte/src/internal/client/dev/ownership.js index 68933d32ece7..367fea9d2c6b 100644 --- a/packages/svelte/src/internal/client/dev.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,6 +1,6 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { deep_read, set_current_owner_override, untrack } from './runtime.js'; +import { deep_read, set_current_owner_override, untrack } from '../runtime.js'; /** @type {Record>} */ const boundaries = {}; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index f524d4fee18b..7cc94082ae68 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { get_component } from './dev.js'; +import { get_component } from './dev/ownership.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 51bca59e3b59..9b2b6d5bce41 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -40,7 +40,7 @@ export { freeze, init } from './client/runtime.js'; -export * from './client/dev.js'; +export * from './client/dev/ownership.js'; export { await_block as await } from './client/dom/blocks/await.js'; export { if_block as if } from './client/dom/blocks/if.js'; export { key_block as key } from './client/dom/blocks/key.js'; From 551f50042f82afcd1f1aaf659cf7f97f1ccb14fd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 17:00:24 -0500 Subject: [PATCH 28/46] rename functions --- .../compiler/phases/3-transform/client/transform-client.js | 4 ++-- packages/svelte/src/internal/client/dev/ownership.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 63feebeda125..764873febcf8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -439,8 +439,8 @@ export function client_component(source, analysis, options) { } if (options.dev && state.options.filename) { - body.unshift(b.stmt(b.call(b.id('$.push_module'), b.id(analysis.name)))); - body.push(b.stmt(b.call(b.id('$.pop_module')))); + body.unshift(b.stmt(b.call(b.id('$.mark_module_start'), b.id(analysis.name)))); + body.push(b.stmt(b.call(b.id('$.mark_module_end')))); } return { diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 367fea9d2c6b..a88d99d5c5cd 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -50,7 +50,7 @@ export function get_component() { /** * @param {Function} component */ -export function push_module(component) { +export function mark_module_start(component) { const start = get_stack()?.[2]; if (start) { @@ -63,7 +63,7 @@ export function push_module(component) { } } -export function pop_module() { +export function mark_module_end() { const end = get_stack()?.[2]; if (end) { From 560b856ac632b5f8b02dc3b49263431785f5ad43 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 17:02:51 -0500 Subject: [PATCH 29/46] add some comments --- packages/svelte/src/internal/client/dev/ownership.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index a88d99d5c5cd..d4d970519534 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -29,6 +29,10 @@ export function get_stack() { return entries; } +/** + * Determines which `.svelte` component is responsible for a given state change + * @returns {Function | null} + */ export function get_component() { const stack = get_stack(); if (!stack) return null; @@ -48,6 +52,9 @@ export function get_component() { } /** + * Together with `mark_module_end`, this function establishes the boundaries of a `.svelte` file, + * such that subsequent calls to `get_component` can tell us which component is responsible + * for a given state change * @param {Function} component */ export function mark_module_start(component) { From 48579dee4ef5f651a1cdbf0fc8f46476cd6fdc4c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 17:11:31 -0500 Subject: [PATCH 30/46] move ownership checking logic --- .../src/internal/client/dev/ownership.js | 21 +++++++++++++++++ .../svelte/src/internal/client/runtime.js | 23 +++++++------------ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index d4d970519534..8218d8103af1 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -91,3 +91,24 @@ export function add_owner(object, owner) { set_current_owner_override(null); }); } + +/** + * @param {import('../types.js').Signal} signal + */ +export function check_ownership(signal) { + // @ts-expect-error + if (!signal.owners) return; + + const component = get_component(); + + // @ts-expect-error + if (component && signal.owners.size > 0 && !signal.owners.has(component)) { + // eslint-disable-next-line no-console + console.warn( + 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' + ); + + // eslint-disable-next-line no-console + console.trace(); + } +} diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 7cc94082ae68..ae3634eeed9b 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { get_component } from './dev/ownership.js'; +import { check_ownership, get_component } from './dev/ownership.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -111,7 +111,11 @@ export let current_component_context = null; export let updating_derived = false; -/** @type {Function | null} */ +/** + * Used to assign ownership to signals in dev mode + * — see `./dev/ownership.js` for more details + * @type {Function | null} + */ export let current_owner = null; /** @@ -1037,19 +1041,8 @@ export function get(signal) { * @returns {V} */ export function set(signal, value) { - if (DEV && 'owners' in signal) { - const component = get_component(); - - // @ts-expect-error - if (component && signal.owners.size > 0 && !signal.owners.has(component)) { - // eslint-disable-next-line no-console - console.warn( - 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' - ); - - // eslint-disable-next-line no-console - console.trace(); - } + if (DEV) { + check_ownership(signal); } set_signal_value(signal, value); From bde311b234a075766e859bf42d6223bddddd5250 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 17:17:24 -0500 Subject: [PATCH 31/46] ugh eslint --- packages/svelte/src/internal/client/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index ae3634eeed9b..9286bd4ed314 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -113,7 +113,7 @@ export let updating_derived = false; /** * Used to assign ownership to signals in dev mode - * — see `./dev/ownership.js` for more details + * — see `./dev/ownership.js` for more details * @type {Function | null} */ export let current_owner = null; From 9f83a0f207ad52f6851eea7cfef1d462262a759c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 18:00:31 -0500 Subject: [PATCH 32/46] more detailed message --- .../phases/3-transform/client/transform-client.js | 12 ++++++++++++ packages/svelte/src/internal/client/dev/ownership.js | 12 +++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 764873febcf8..4a0b2e6e0292 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -346,6 +346,18 @@ export function client_component(source, analysis, options) { ) ]; + if (options.filename) { + body.push( + b.stmt( + b.assignment( + '=', + b.member(b.id(analysis.name), b.id('filename')), + b.literal(options.filename) + ) + ) + ); + } + if (options.discloseVersion) { body.unshift(b.imports([], 'svelte/internal/disclose-version')); } diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 8218d8103af1..937553f8fbd8 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -103,9 +103,19 @@ export function check_ownership(signal) { // @ts-expect-error if (component && signal.owners.size > 0 && !signal.owners.has(component)) { + // @ts-expect-error + let owner = [...signal.owners][0]; + + let message = + // @ts-expect-error + owner.filename !== component.filename + ? // @ts-expect-error + `${component.filename} mutated a value owned by ${owner.filename}. This is strongly discouraged` + : 'Mutating a value outside the component that created it is strongly discouraged'; + // eslint-disable-next-line no-console console.warn( - 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' + `${message}. Consider passing values to child components with \`bind:\`, or use a callback instead.` ); // eslint-disable-next-line no-console From 954da01b6a96a223cf54effd7bb2d6d3cef5437d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 18:00:57 -0500 Subject: [PATCH 33/46] only add filename in dev --- .../src/compiler/phases/3-transform/client/transform-client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 4a0b2e6e0292..189508d67aa9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -346,7 +346,7 @@ export function client_component(source, analysis, options) { ) ]; - if (options.filename) { + if (options.dev && options.filename) { body.push( b.stmt( b.assignment( From cd972d9779486376f5454697df5fc35eca6bdc85 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 18:01:24 -0500 Subject: [PATCH 34/46] comment --- .../src/compiler/phases/3-transform/client/transform-client.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 189508d67aa9..0eac2dc5f0f8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -347,6 +347,7 @@ export function client_component(source, analysis, options) { ]; if (options.dev && options.filename) { + // add `App.filename = 'App.svelte'` so that we can print useful messages later body.push( b.stmt( b.assignment( From f4f3d3a8308457e9ae6eb0c0282aa9b9153c2cb0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 18:17:15 -0500 Subject: [PATCH 35/46] update tests --- packages/svelte/tests/helpers.js | 6 +++--- .../samples/non-local-mutation-discouraged/_config.js | 2 +- .../samples/bind-this/_expected/client/index.svelte.js | 2 +- .../samples/bind-this/_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/server/index.svelte.js | 2 +- .../each-string-template/_expected/client/index.svelte.js | 4 ++-- .../each-string-template/_expected/server/index.svelte.js | 2 +- .../samples/export-state/_expected/client/index.svelte.js | 2 +- .../samples/export-state/_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 4 ++-- .../_expected/server/index.svelte.js | 2 +- .../samples/hello-world/_expected/client/index.svelte.js | 4 ++-- .../samples/hello-world/_expected/server/index.svelte.js | 2 +- .../state-proxy-literal/_expected/client/index.svelte.js | 2 +- .../state-proxy-literal/_expected/server/index.svelte.js | 2 +- .../svelte-element/_expected/client/index.svelte.js | 2 +- .../svelte-element/_expected/server/index.svelte.js | 2 +- .../tests/sourcemaps/samples/css-injected-map/_config.js | 2 +- .../tests/sourcemaps/samples/sourcemap-sources/_config.js | 8 +------- .../tests/sourcemaps/samples/static-no-script/_config.js | 2 +- packages/svelte/tests/sourcemaps/test.ts | 4 ++-- 22 files changed, 28 insertions(+), 34 deletions(-) diff --git a/packages/svelte/tests/helpers.js b/packages/svelte/tests/helpers.js index d08b81b5701c..8bf56b8fa9fc 100644 --- a/packages/svelte/tests/helpers.js +++ b/packages/svelte/tests/helpers.js @@ -73,7 +73,7 @@ export async function compile_directory( let text = fs.readFileSync(`${cwd}/${file}`, 'utf-8'); let opts = { - filename: path.join(cwd, file), + filename: file, ...compileOptions, generate }; @@ -117,8 +117,8 @@ export async function compile_directory( } const compiled = compile(text, { - outputFilename: `${output_dir}/${file}${file.endsWith('.js') ? '' : '.js'}`, - cssOutputFilename: `${output_dir}/${file}.css`, + outputFilename: `${file}${file.endsWith('.js') ? '' : '.js'}`, + cssOutputFilename: `${file}.css`, ...opts }); compiled.js.code = compiled.js.code.replace(`v${VERSION}`, 'VERSION'); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js index 81b3782158f9..37b5394f81a3 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js @@ -41,7 +41,7 @@ export default test({ assert.htmlEqual(target.innerHTML, ``); assert.deepEqual(warnings, [ - 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use callback instead.' + 'Counter.svelte mutated a value owned by main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.' ]); } }); diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js index 0f261e1c81c9..33136e6faa65 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Bind_this($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, false); $.init(); diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js index 71b688fe3785..838d15f2a5bf 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Bind_this($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(false); const anchor = $.create_anchor($$payload); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index 38a74ef9bb87..99d04e98a365 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Class_state_field_constructor_assignment($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, true); class Foo { diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js index 6ba11ebb1619..4d99f0e9e717 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Class_state_field_constructor_assignment($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(true); class Foo { diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js index 7bf677e7e9ad..8026752a3c79 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Each_string_template($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, false); $.init(); @@ -28,4 +28,4 @@ export default function Each_string_template($$anchor, $$props) { $.close_frag($$anchor, fragment); $.pop(); -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js index b23b316e3d7b..1c23a0c032f1 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Each_string_template($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(false); const anchor = $.create_anchor($$payload); diff --git a/packages/svelte/tests/snapshot/samples/export-state/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/export-state/_expected/client/index.svelte.js index 4cc594e9d06c..c9ba1e7c7334 100644 --- a/packages/svelte/tests/snapshot/samples/export-state/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/export-state/_expected/client/index.svelte.js @@ -1,4 +1,4 @@ /* index.svelte.js generated by Svelte VERSION */ import * as $ from "svelte/internal"; -export const object = $.proxy({ ok: true }); \ No newline at end of file +export const object = $.proxy({ ok: true }); diff --git a/packages/svelte/tests/snapshot/samples/export-state/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/export-state/_expected/server/index.svelte.js index a3b619df6eef..d7bc4c8f334d 100644 --- a/packages/svelte/tests/snapshot/samples/export-state/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/export-state/_expected/server/index.svelte.js @@ -1,4 +1,4 @@ /* index.svelte.js generated by Svelte VERSION */ import * as $ from "svelte/internal/server"; -export const object = { ok: true }; \ No newline at end of file +export const object = { ok: true }; diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index 12e7295f6a08..db959fdee415 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Function_prop_no_getter($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, true); let count = $.source(0); @@ -33,4 +33,4 @@ export default function Function_prop_no_getter($$anchor, $$props) { $.close_frag($$anchor, fragment); $.pop(); -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js index f678bb6ad67d..6c0d2e9922dd 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Function_prop_no_getter($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(true); let count = 0; diff --git a/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js index 7eb39880c4df..749e8abd40ba 100644 --- a/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js @@ -5,7 +5,7 @@ import * as $ from "svelte/internal"; var frag = $.template(`

hello world

`); -export default function Hello_world($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, false); $.init(); @@ -14,4 +14,4 @@ export default function Hello_world($$anchor, $$props) { $.close($$anchor, h1); $.pop(); -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js index 7007fe2b48de..1e9394f59ab2 100644 --- a/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Hello_world($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(false); $$payload.out += `

hello world

`; $.pop(); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js index 3da681ea5394..984b485431c9 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js @@ -12,7 +12,7 @@ function reset(_, str, tpl) { var frag = $.template(` `, true); -export default function State_proxy_literal($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, true); let str = $.source(''); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js index c541299e36a7..69e294b76db5 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function State_proxy_literal($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(true); let str = ''; diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js index b2734b8c4648..bcb630f92867 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Svelte_element($$anchor, $$props) { +export default function Index($$anchor, $$props) { $.push($$props, true); let tag = $.prop($$props, "tag", 3, 'hr'); diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js index 405a45227197..51bb86c2b236 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Svelte_element($$payload, $$props) { +export default function Index($$payload, $$props) { $.push(true); let { tag = 'hr' } = $$props; diff --git a/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js b/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js index 2699d350a8b8..ff2ec2e0c5bc 100644 --- a/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js @@ -42,7 +42,7 @@ export default test({ const css_map_json = Buffer.from(css_map_base64, 'base64').toString(); const map = new TraceMap(css_map_json); - const sourcefile = '../../input.svelte'; + const sourcefile = 'input.svelte'; const locate = getLocator( css.replace(/\\r/g, '\r').replace(/\\n/g, '\n').replace(/\\t/g, '\t'), { offsetLine: 1 } diff --git a/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js b/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js index 4c72229ea65f..8a15daef7aa6 100644 --- a/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js @@ -38,13 +38,7 @@ const FOO2 = 'var answer2 = 84; // foo2.js\n'; const BAR2 = 'console.log(answer2); // bar2.js\n'; export default test({ - js_map_sources: [ - '../../input.svelte', - '../../foo.js', - '../../bar.js', - '../../foo2.js', - '../../bar2.js' - ], + js_map_sources: ['input.svelte', 'foo.js', 'bar.js', 'foo2.js', 'bar2.js'], preprocess: [ { script: ({ content, filename = '' }) => { diff --git a/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js b/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js index 5cd3b335c993..268ed3c57eda 100644 --- a/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js @@ -2,7 +2,7 @@ import { test } from '../../test'; export default test({ test({ assert, map_client }) { - assert.deepEqual(map_client.sources, ['../../input.svelte']); + assert.deepEqual(map_client.sources, ['input.svelte']); // TODO do we need to set sourcesContent? We did it in Svelte 4, but why? // assert.deepEqual(js.map.sourcesContent, [ // fs.readFileSync(path.join(__dirname, 'input.svelte'), 'utf-8') diff --git a/packages/svelte/tests/sourcemaps/test.ts b/packages/svelte/tests/sourcemaps/test.ts index c654258b34fd..8ce4b9a4fc20 100644 --- a/packages/svelte/tests/sourcemaps/test.ts +++ b/packages/svelte/tests/sourcemaps/test.ts @@ -198,7 +198,7 @@ const { test, run } = suite(async (config, cwd) => { map_client = JSON.parse(fs.readFileSync(`${cwd}/_output/client/input.svelte.js.map`, 'utf-8')); assert.deepEqual( map_client.sources.slice().sort(), - (config.js_map_sources || ['../../input.svelte']).sort(), + (config.js_map_sources || ['input.svelte']).sort(), 'js.map.sources is wrong' ); @@ -238,7 +238,7 @@ const { test, run } = suite(async (config, cwd) => { map_css = JSON.parse(fs.readFileSync(`${cwd}/_output/client/input.svelte.css.map`, 'utf-8')); assert.deepEqual( map_css.sources.slice().sort(), - (config.css_map_sources || ['../../input.svelte']).sort(), + (config.css_map_sources || ['input.svelte']).sort(), 'css.map.sources is wrong' ); compare('css', code_css, map_css, config.css); From fc40b79494947375015cc18233d8e19c65e08efa Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Feb 2024 19:51:25 -0500 Subject: [PATCH 36/46] move more code --- .../src/internal/client/dev/ownership.js | 34 +++++++++++++++++-- .../svelte/src/internal/client/runtime.js | 27 ++------------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 937553f8fbd8..c42d8ed494b3 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,6 +1,6 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { deep_read, set_current_owner_override, untrack } from '../runtime.js'; +import { current_component_context, current_owner, deep_read, untrack } from '../runtime.js'; /** @type {Record>} */ const boundaries = {}; @@ -79,6 +79,20 @@ export function mark_module_end() { } } +/** @type {Function | null} */ +let new_owner = null; + +/** + * @param {import('../types.js').Signal} signal + */ +export function set_owner(signal) { + // @ts-expect-error + if (current_owner && signal.owners) { + // @ts-expect-error + signal.owners.add(current_owner); + } +} + /** * * @param {any} object @@ -86,12 +100,26 @@ export function mark_module_end() { */ export function add_owner(object, owner) { untrack(() => { - set_current_owner_override(owner); + new_owner = owner; deep_read(object); - set_current_owner_override(null); + new_owner = null; }); } +/** + * @param {import('../types.js').Signal} signal + */ +export function add_owner_to_signal(signal) { + if ( + new_owner && + // @ts-expect-error + signal.owners?.has(current_component_context.function) + ) { + // @ts-expect-error + signal.owners.add(new_owner); + } +} + /** * @param {import('../types.js').Signal} signal */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 9286bd4ed314..9b8b6f88dcf7 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { check_ownership, get_component } from './dev/ownership.js'; +import { add_owner_to_signal, check_ownership, get_component, set_owner } from './dev/ownership.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -125,16 +125,6 @@ export function set_current_owner(owner) { current_owner = owner; } -/** @type {Function | null} */ -export let current_owner_override = null; - -/** - * @param {Function | null} owner - */ -export function set_current_owner_override(owner) { - current_owner_override = owner; -} - /** * @param {null | import('./types.js').ComponentContext} context * @returns {boolean} @@ -961,14 +951,7 @@ export function unsubscribe_on_destroy(stores) { */ export function get(signal) { if (DEV) { - if ( - current_owner_override && - // @ts-expect-error - signal.owners?.has(current_component_context.function) - ) { - // @ts-expect-error - signal.owners.add(current_owner_override); - } + add_owner_to_signal(signal); // @ts-expect-error if (signal.inspect && inspect_fn) { @@ -1361,11 +1344,7 @@ export function source(initial_value) { */ function bind_signal_to_component_context(signal) { if (DEV) { - // @ts-expect-error - if (current_owner && signal.owners) { - // @ts-expect-error - signal.owners.add(current_owner); - } + set_owner(signal); } if (current_component_context === null || !current_component_context.r) return; From adf542a8e27d5c1ba223801481431b1576372bf2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 06:19:09 -0500 Subject: [PATCH 37/46] undo change to sourcemap tests --- packages/svelte/tests/helpers.js | 4 ++-- .../tests/sourcemaps/samples/css-injected-map/_config.js | 2 +- .../tests/sourcemaps/samples/sourcemap-sources/_config.js | 8 +++++++- .../tests/sourcemaps/samples/static-no-script/_config.js | 2 +- packages/svelte/tests/sourcemaps/test.ts | 4 ++-- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/svelte/tests/helpers.js b/packages/svelte/tests/helpers.js index 8bf56b8fa9fc..5f85c7f59f99 100644 --- a/packages/svelte/tests/helpers.js +++ b/packages/svelte/tests/helpers.js @@ -117,8 +117,8 @@ export async function compile_directory( } const compiled = compile(text, { - outputFilename: `${file}${file.endsWith('.js') ? '' : '.js'}`, - cssOutputFilename: `${file}.css`, + outputFilename: `_output/${generate}/${file}${file.endsWith('.js') ? '' : '.js'}`, + cssOutputFilename: `_output/${generate}/${file}.css`, ...opts }); compiled.js.code = compiled.js.code.replace(`v${VERSION}`, 'VERSION'); diff --git a/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js b/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js index ff2ec2e0c5bc..2699d350a8b8 100644 --- a/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/css-injected-map/_config.js @@ -42,7 +42,7 @@ export default test({ const css_map_json = Buffer.from(css_map_base64, 'base64').toString(); const map = new TraceMap(css_map_json); - const sourcefile = 'input.svelte'; + const sourcefile = '../../input.svelte'; const locate = getLocator( css.replace(/\\r/g, '\r').replace(/\\n/g, '\n').replace(/\\t/g, '\t'), { offsetLine: 1 } diff --git a/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js b/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js index 8a15daef7aa6..4c72229ea65f 100644 --- a/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/sourcemap-sources/_config.js @@ -38,7 +38,13 @@ const FOO2 = 'var answer2 = 84; // foo2.js\n'; const BAR2 = 'console.log(answer2); // bar2.js\n'; export default test({ - js_map_sources: ['input.svelte', 'foo.js', 'bar.js', 'foo2.js', 'bar2.js'], + js_map_sources: [ + '../../input.svelte', + '../../foo.js', + '../../bar.js', + '../../foo2.js', + '../../bar2.js' + ], preprocess: [ { script: ({ content, filename = '' }) => { diff --git a/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js b/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js index 268ed3c57eda..5cd3b335c993 100644 --- a/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/static-no-script/_config.js @@ -2,7 +2,7 @@ import { test } from '../../test'; export default test({ test({ assert, map_client }) { - assert.deepEqual(map_client.sources, ['input.svelte']); + assert.deepEqual(map_client.sources, ['../../input.svelte']); // TODO do we need to set sourcesContent? We did it in Svelte 4, but why? // assert.deepEqual(js.map.sourcesContent, [ // fs.readFileSync(path.join(__dirname, 'input.svelte'), 'utf-8') diff --git a/packages/svelte/tests/sourcemaps/test.ts b/packages/svelte/tests/sourcemaps/test.ts index 8ce4b9a4fc20..c654258b34fd 100644 --- a/packages/svelte/tests/sourcemaps/test.ts +++ b/packages/svelte/tests/sourcemaps/test.ts @@ -198,7 +198,7 @@ const { test, run } = suite(async (config, cwd) => { map_client = JSON.parse(fs.readFileSync(`${cwd}/_output/client/input.svelte.js.map`, 'utf-8')); assert.deepEqual( map_client.sources.slice().sort(), - (config.js_map_sources || ['input.svelte']).sort(), + (config.js_map_sources || ['../../input.svelte']).sort(), 'js.map.sources is wrong' ); @@ -238,7 +238,7 @@ const { test, run } = suite(async (config, cwd) => { map_css = JSON.parse(fs.readFileSync(`${cwd}/_output/client/input.svelte.css.map`, 'utf-8')); assert.deepEqual( map_css.sources.slice().sort(), - (config.css_map_sources || ['input.svelte']).sort(), + (config.css_map_sources || ['../../input.svelte']).sort(), 'css.map.sources is wrong' ); compare('css', code_css, map_css, config.css); From b025d483159aabef4cb75b193d85226370e68857 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 06:55:05 -0500 Subject: [PATCH 38/46] allow proxy to have multiple owners --- .../src/internal/client/dev/ownership.js | 12 +++++++----- packages/svelte/src/internal/client/proxy.js | 12 ++++++------ packages/svelte/src/internal/client/runtime.js | 18 +++++++++--------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index c42d8ed494b3..c34859e2f8d6 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,6 +1,6 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { current_component_context, current_owner, deep_read, untrack } from '../runtime.js'; +import { current_component_context, current_owners, deep_read, untrack } from '../runtime.js'; /** @type {Record>} */ const boundaries = {}; @@ -85,11 +85,13 @@ let new_owner = null; /** * @param {import('../types.js').Signal} signal */ -export function set_owner(signal) { +export function set_owners(signal) { // @ts-expect-error - if (current_owner && signal.owners) { - // @ts-expect-error - signal.owners.add(current_owner); + if (current_owners && signal.owners) { + for (const owner of current_owners) { + // @ts-expect-error + signal.owners.add(owner); + } } } diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 0039edd5a2d4..fca19d24a2f4 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -9,8 +9,8 @@ import { UNINITIALIZED, mutable_source, batch_inspect, - set_current_owner, - current_owner + set_current_owners, + current_owners } from './runtime.js'; import { array_prototype, @@ -62,7 +62,7 @@ export function proxy(value, immutable = true) { if (DEV) { // @ts-expect-error - value[STATE_SYMBOL].owner = current_owner; + value[STATE_SYMBOL].owner = new Set(current_owners); } return proxy; @@ -183,10 +183,10 @@ const state_proxy_handler = { (effect_active() || updating_derived) && (!(prop in target) || get_descriptor(target, prop)?.writable) ) { - const previous_owner = current_owner; - if (DEV) set_current_owner(metadata.owner); + const previous_owners = current_owners; + if (DEV) set_current_owners(metadata.owner); s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); - if (DEV) set_current_owner(previous_owner); + if (DEV) set_current_owners(previous_owners); metadata.s.set(prop, s); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 9b8b6f88dcf7..01839dc6a6d5 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,7 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { add_owner_to_signal, check_ownership, get_component, set_owner } from './dev/ownership.js'; +import { add_owner_to_signal, check_ownership, set_owners } from './dev/ownership.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -114,15 +114,15 @@ export let updating_derived = false; /** * Used to assign ownership to signals in dev mode * — see `./dev/ownership.js` for more details - * @type {Function | null} + * @type {Function[] | null} */ -export let current_owner = null; +export let current_owners = null; /** - * @param {Function | null} owner + * @param {Function[] | null} owners */ -export function set_current_owner(owner) { - current_owner = owner; +export function set_current_owners(owners) { + current_owners = owners; } /** @@ -1344,7 +1344,7 @@ export function source(initial_value) { */ function bind_signal_to_component_context(signal) { if (DEV) { - set_owner(signal); + set_owners(signal); } if (current_component_context === null || !current_component_context.r) return; @@ -1935,7 +1935,7 @@ export function push(props, runes = false, fn) { if (DEV) { // component function // @ts-expect-error - current_owner = current_component_context.function = fn; + current_owners = [(current_component_context.function = fn)]; } } @@ -1960,7 +1960,7 @@ export function pop(component) { current_component_context = context_stack_item.p; if (DEV) { // @ts-expect-error - current_owner = current_component_context?.function; + current_owners = current_component_context ? [current_component_context.function] : null; } context_stack_item.m = true; } From 8d2a7d07d6e5b76719c92ed14f5b5725852863e1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 08:32:51 -0500 Subject: [PATCH 39/46] use SignalDebug --- packages/svelte/src/internal/client/dev/ownership.js | 12 +++--------- packages/svelte/src/internal/client/runtime.js | 10 ++++++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index c34859e2f8d6..5f9ede212eb5 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -83,13 +83,11 @@ export function mark_module_end() { let new_owner = null; /** - * @param {import('../types.js').Signal} signal + * @param {import('../types.js').SignalDebug} signal */ export function set_owners(signal) { - // @ts-expect-error if (current_owners && signal.owners) { for (const owner of current_owners) { - // @ts-expect-error signal.owners.add(owner); } } @@ -109,7 +107,7 @@ export function add_owner(object, owner) { } /** - * @param {import('../types.js').Signal} signal + * @param {import('../types.js').SignalDebug} signal */ export function add_owner_to_signal(signal) { if ( @@ -117,23 +115,19 @@ export function add_owner_to_signal(signal) { // @ts-expect-error signal.owners?.has(current_component_context.function) ) { - // @ts-expect-error signal.owners.add(new_owner); } } /** - * @param {import('../types.js').Signal} signal + * @param {import('../types.js').SignalDebug} signal */ export function check_ownership(signal) { - // @ts-expect-error if (!signal.owners) return; const component = get_component(); - // @ts-expect-error if (component && signal.owners.size > 0 && !signal.owners.has(component)) { - // @ts-expect-error let owner = [...signal.owners][0]; let message = diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 01839dc6a6d5..d84751bbc656 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -951,11 +951,13 @@ export function unsubscribe_on_destroy(stores) { */ export function get(signal) { if (DEV) { - add_owner_to_signal(signal); + const debuggable = /** @type {import('./types.js').SignalDebug} */ (signal); + + add_owner_to_signal(debuggable); // @ts-expect-error if (signal.inspect && inspect_fn) { - /** @type {import('./types.js').SignalDebug} */ (signal).inspect.add(inspect_fn); + debuggable.inspect.add(inspect_fn); // @ts-expect-error inspect_captured_signals.push(signal); } @@ -1025,7 +1027,7 @@ export function get(signal) { */ export function set(signal, value) { if (DEV) { - check_ownership(signal); + check_ownership(/** @type {import('./types.js').SignalDebug} */ (signal)); } set_signal_value(signal, value); @@ -1344,7 +1346,7 @@ export function source(initial_value) { */ function bind_signal_to_component_context(signal) { if (DEV) { - set_owners(signal); + set_owners(/** @type {import('./types.js').SignalDebug} */ (signal)); } if (current_component_context === null || !current_component_context.r) return; From 9dc3205816133f6a1b8694cd0ffab37e02df7d2d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 09:05:04 -0500 Subject: [PATCH 40/46] i was doing this all wrong --- .../src/internal/client/dev/ownership.js | 47 +++++++------------ packages/svelte/src/internal/client/proxy.js | 31 +++++++++--- .../svelte/src/internal/client/runtime.js | 14 +----- .../svelte/src/internal/client/types.d.ts | 6 +-- 4 files changed, 45 insertions(+), 53 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 5f9ede212eb5..909548712154 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,5 +1,6 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ +import { STATE_SYMBOL } from '../proxy.js'; import { current_component_context, current_owners, deep_read, untrack } from '../runtime.js'; /** @type {Record>} */ @@ -82,17 +83,6 @@ export function mark_module_end() { /** @type {Function | null} */ let new_owner = null; -/** - * @param {import('../types.js').SignalDebug} signal - */ -export function set_owners(signal) { - if (current_owners && signal.owners) { - for (const owner of current_owners) { - signal.owners.add(owner); - } - } -} - /** * * @param {any} object @@ -100,41 +90,38 @@ export function set_owners(signal) { */ export function add_owner(object, owner) { untrack(() => { - new_owner = owner; - deep_read(object); - new_owner = null; + add_owner_to_object(object, owner); }); } /** - * @param {import('../types.js').SignalDebug} signal + * @param {any} object + * @param {Function} owner */ -export function add_owner_to_signal(signal) { - if ( - new_owner && - // @ts-expect-error - signal.owners?.has(current_component_context.function) - ) { - signal.owners.add(new_owner); +function add_owner_to_object(object, owner) { + if (object?.[STATE_SYMBOL]?.owners && !object[STATE_SYMBOL].owners.has(owner)) { + object[STATE_SYMBOL].owners.add(owner); + + for (const key in object) { + add_owner_to_object(object[key], owner); + } } } /** - * @param {import('../types.js').SignalDebug} signal + * @param {Set} owners */ -export function check_ownership(signal) { - if (!signal.owners) return; - +export function check_ownership(owners) { const component = get_component(); - if (component && signal.owners.size > 0 && !signal.owners.has(component)) { - let owner = [...signal.owners][0]; + if (component && !owners.has(component)) { + let original = [...owners][0]; let message = // @ts-expect-error - owner.filename !== component.filename + original.filename !== component.filename ? // @ts-expect-error - `${component.filename} mutated a value owned by ${owner.filename}. This is strongly discouraged` + `${component.filename} mutated a value owned by ${original.filename}. This is strongly discouraged` : 'Mutating a value outside the component that created it is strongly discouraged'; // eslint-disable-next-line no-console diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index fca19d24a2f4..a689dcd44172 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -10,7 +10,8 @@ import { mutable_source, batch_inspect, set_current_owners, - current_owners + current_owners, + current_component_context } from './runtime.js'; import { array_prototype, @@ -22,6 +23,7 @@ import { is_frozen, object_prototype } from './utils.js'; +import { check_ownership } from './dev/ownership.js'; export const STATE_SYMBOL = Symbol('$state'); @@ -29,9 +31,10 @@ export const STATE_SYMBOL = Symbol('$state'); * @template T * @param {T} value * @param {boolean} [immutable] + * @param {Function[]} [owners] * @returns {import('./types.js').ProxyStateObject | T} */ -export function proxy(value, immutable = true) { +export function proxy(value, immutable = true, owners) { if (typeof value === 'object' && value != null && !is_frozen(value)) { // If we have an existing proxy, return it... if (STATE_SYMBOL in value) { @@ -62,7 +65,13 @@ export function proxy(value, immutable = true) { if (DEV) { // @ts-expect-error - value[STATE_SYMBOL].owner = new Set(current_owners); + value[STATE_SYMBOL].owners = + owners === undefined + ? current_component_context + ? // @ts-expect-error + new Set([current_component_context.function]) + : null + : new Set(owners); } return proxy; @@ -136,7 +145,7 @@ const state_proxy_handler = { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(descriptor.value, metadata.i)); + if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata.owners)); } return Reflect.defineProperty(target, prop, descriptor); @@ -185,7 +194,7 @@ const state_proxy_handler = { ) { const previous_owners = current_owners; if (DEV) set_current_owners(metadata.owner); - s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i)); + s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata.owners)); if (DEV) set_current_owners(previous_owners); metadata.s.set(prop, s); } @@ -228,7 +237,7 @@ const state_proxy_handler = { if (s !== undefined || (effect_active() && (!has || get_descriptor(target, prop)?.writable))) { if (s === undefined) { s = (metadata.i ? source : mutable_source)( - has ? proxy(target[prop], metadata.i) : UNINITIALIZED + has ? proxy(target[prop], metadata.i, metadata.owners) : UNINITIALIZED ); metadata.s.set(prop, s); } @@ -243,10 +252,18 @@ const state_proxy_handler = { set(target, prop, value) { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(value, metadata.i)); + if (s !== undefined) { + set(s, proxy(value, metadata.i, metadata.owners)); + } else if (DEV) { + // TODO transfer ownership, in case it differs + } const is_array = metadata.a; const not_has = !(prop in target); + if (DEV && metadata.owners) { + check_ownership(metadata.owners); + } + // variable.length = value -> clear all signals with index >= value if (is_array && prop === 'length') { for (let i = value; i < target.length; i += 1) { diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index d84751bbc656..b612562bf6bd 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -19,7 +19,6 @@ import { } from '../../constants.js'; import { STATE_SYMBOL, unstate } from './proxy.js'; import { EACH_BLOCK, IF_BLOCK } from './block.js'; -import { add_owner_to_signal, check_ownership, set_owners } from './dev/ownership.js'; export const SOURCE = 1; export const DERIVED = 1 << 1; @@ -183,8 +182,7 @@ function create_source_signal(flags, value) { // value v: value, // this is for DEV only - inspect: new Set(), - owners: new Set() + inspect: new Set() }; } return { @@ -953,8 +951,6 @@ export function get(signal) { if (DEV) { const debuggable = /** @type {import('./types.js').SignalDebug} */ (signal); - add_owner_to_signal(debuggable); - // @ts-expect-error if (signal.inspect && inspect_fn) { debuggable.inspect.add(inspect_fn); @@ -1026,10 +1022,6 @@ export function get(signal) { * @returns {V} */ export function set(signal, value) { - if (DEV) { - check_ownership(/** @type {import('./types.js').SignalDebug} */ (signal)); - } - set_signal_value(signal, value); return value; } @@ -1345,10 +1337,6 @@ export function source(initial_value) { * @param {import('./types.js').Signal} signal */ function bind_signal_to_component_context(signal) { - if (DEV) { - set_owners(/** @type {import('./types.js').SignalDebug} */ (signal)); - } - if (current_component_context === null || !current_component_context.r) return; const signals = current_component_context.d; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 58fe73618d25..3d36fe932660 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -83,7 +83,7 @@ export type SourceSignalDebug = { /** This is DEV only */ inspect: Set; /** This is DEV only */ - owners: Set; + proxy?: ProxyStateObject; }; export type ComputationSignal = { @@ -408,8 +408,8 @@ export interface ProxyMetadata> { p: ProxyStateObject; /** The original target this proxy was created for */ t: T; - /** The component that 'owns' this state, if any */ - o: string; + /** Dev-only — the components that 'own' this state, if any */ + owners: null | Set; } export type ProxyStateObject> = T & { From 6dd301d8e90286b3cac7533b59c424c6e1fed1d0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 09:47:35 -0500 Subject: [PATCH 41/46] tidy up --- .../src/internal/client/dev/ownership.js | 9 +++------ packages/svelte/src/internal/client/proxy.js | 19 +++++++----------- .../svelte/src/internal/client/runtime.js | 20 +------------------ .../svelte/src/internal/client/types.d.ts | 4 +--- 4 files changed, 12 insertions(+), 40 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 909548712154..ac2b94fd09ab 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,7 +1,7 @@ /** @typedef {{ file: string, line: number, column: number }} Location */ import { STATE_SYMBOL } from '../proxy.js'; -import { current_component_context, current_owners, deep_read, untrack } from '../runtime.js'; +import { untrack } from '../runtime.js'; /** @type {Record>} */ const boundaries = {}; @@ -80,9 +80,6 @@ export function mark_module_end() { } } -/** @type {Function | null} */ -let new_owner = null; - /** * * @param {any} object @@ -99,8 +96,8 @@ export function add_owner(object, owner) { * @param {Function} owner */ function add_owner_to_object(object, owner) { - if (object?.[STATE_SYMBOL]?.owners && !object[STATE_SYMBOL].owners.has(owner)) { - object[STATE_SYMBOL].owners.add(owner); + if (object?.[STATE_SYMBOL]?.o && !object[STATE_SYMBOL].o.has(owner)) { + object[STATE_SYMBOL].o.add(owner); for (const key in object) { add_owner_to_object(object[key], owner); diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index a689dcd44172..587c44bb2eb3 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -9,8 +9,6 @@ import { UNINITIALIZED, mutable_source, batch_inspect, - set_current_owners, - current_owners, current_component_context } from './runtime.js'; import { @@ -65,7 +63,7 @@ export function proxy(value, immutable = true, owners) { if (DEV) { // @ts-expect-error - value[STATE_SYMBOL].owners = + value[STATE_SYMBOL].o = owners === undefined ? current_component_context ? // @ts-expect-error @@ -145,7 +143,7 @@ const state_proxy_handler = { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata.owners)); + if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata.o)); } return Reflect.defineProperty(target, prop, descriptor); @@ -192,10 +190,7 @@ const state_proxy_handler = { (effect_active() || updating_derived) && (!(prop in target) || get_descriptor(target, prop)?.writable) ) { - const previous_owners = current_owners; - if (DEV) set_current_owners(metadata.owner); - s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata.owners)); - if (DEV) set_current_owners(previous_owners); + s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata.o)); metadata.s.set(prop, s); } @@ -237,7 +232,7 @@ const state_proxy_handler = { if (s !== undefined || (effect_active() && (!has || get_descriptor(target, prop)?.writable))) { if (s === undefined) { s = (metadata.i ? source : mutable_source)( - has ? proxy(target[prop], metadata.i, metadata.owners) : UNINITIALIZED + has ? proxy(target[prop], metadata.i, metadata.o) : UNINITIALIZED ); metadata.s.set(prop, s); } @@ -253,15 +248,15 @@ const state_proxy_handler = { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); if (s !== undefined) { - set(s, proxy(value, metadata.i, metadata.owners)); + set(s, proxy(value, metadata.i, metadata.o)); } else if (DEV) { // TODO transfer ownership, in case it differs } const is_array = metadata.a; const not_has = !(prop in target); - if (DEV && metadata.owners) { - check_ownership(metadata.owners); + if (DEV && metadata.o) { + check_ownership(metadata.o); } // variable.length = value -> clear all signals with index >= value diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index b612562bf6bd..476c2483b479 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -110,20 +110,6 @@ export let current_component_context = null; export let updating_derived = false; -/** - * Used to assign ownership to signals in dev mode - * — see `./dev/ownership.js` for more details - * @type {Function[] | null} - */ -export let current_owners = null; - -/** - * @param {Function[] | null} owners - */ -export function set_current_owners(owners) { - current_owners = owners; -} - /** * @param {null | import('./types.js').ComponentContext} context * @returns {boolean} @@ -1925,7 +1911,7 @@ export function push(props, runes = false, fn) { if (DEV) { // component function // @ts-expect-error - current_owners = [(current_component_context.function = fn)]; + current_component_context.function = fn; } } @@ -1948,10 +1934,6 @@ export function pop(component) { } } current_component_context = context_stack_item.p; - if (DEV) { - // @ts-expect-error - current_owners = current_component_context ? [current_component_context.function] : null; - } context_stack_item.m = true; } // Micro-optimization: Don't set .a above to the empty object diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 3d36fe932660..8b49dc51d460 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -82,8 +82,6 @@ export type SourceSignal = { export type SourceSignalDebug = { /** This is DEV only */ inspect: Set; - /** This is DEV only */ - proxy?: ProxyStateObject; }; export type ComputationSignal = { @@ -409,7 +407,7 @@ export interface ProxyMetadata> { /** The original target this proxy was created for */ t: T; /** Dev-only — the components that 'own' this state, if any */ - owners: null | Set; + o: null | Set; } export type ProxyStateObject> = T & { From c6214cee05f8668cc3d2f89eff7d4b43deee65c7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 10:12:44 -0500 Subject: [PATCH 42/46] implement inheritance --- .../src/internal/client/dev/ownership.js | 22 +++++++++++ packages/svelte/src/internal/client/proxy.js | 19 ++++++++-- packages/svelte/tests/helpers.js | 6 ++- .../Counter.svelte | 7 ++++ .../_config.js | 37 +++++++++++++++++++ .../main.svelte | 9 +++++ .../state.svelte.js | 3 ++ 7 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/state.svelte.js diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index ac2b94fd09ab..7d03e3c37f31 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -105,6 +105,28 @@ function add_owner_to_object(object, owner) { } } +/** + * @param {any} object + */ +export function strip_owner(object) { + untrack(() => { + strip_owner_from_object(object); + }); +} + +/** + * @param {any} object + */ +function strip_owner_from_object(object) { + if (object?.[STATE_SYMBOL]?.o) { + object[STATE_SYMBOL].o = null; + + for (const key in object) { + strip_owner(object[key]); + } + } +} + /** * @param {Set} owners */ diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 587c44bb2eb3..9c8c169ee321 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -21,7 +21,7 @@ import { is_frozen, object_prototype } from './utils.js'; -import { check_ownership } from './dev/ownership.js'; +import { add_owner, check_ownership, strip_owner } from './dev/ownership.js'; export const STATE_SYMBOL = Symbol('$state'); @@ -39,7 +39,20 @@ export function proxy(value, immutable = true, owners) { const metadata = /** @type {import('./types.js').ProxyMetadata} */ (value[STATE_SYMBOL]); // ...unless the proxy belonged to a different object, because // someone copied the state symbol using `Reflect.ownKeys(...)` - if (metadata.t === value || metadata.p === value) return metadata.p; + if (metadata.t === value || metadata.p === value) { + if (DEV) { + // update ownership + if (owners) { + for (const owner of owners) { + add_owner(value, owner); + } + } else { + strip_owner(value); + } + } + + return metadata.p; + } } const prototype = get_prototype_of(value); @@ -249,8 +262,6 @@ const state_proxy_handler = { const s = metadata.s.get(prop); if (s !== undefined) { set(s, proxy(value, metadata.i, metadata.o)); - } else if (DEV) { - // TODO transfer ownership, in case it differs } const is_array = metadata.a; const not_has = !(prop in target); diff --git a/packages/svelte/tests/helpers.js b/packages/svelte/tests/helpers.js index 5f85c7f59f99..7b8da31dd28a 100644 --- a/packages/svelte/tests/helpers.js +++ b/packages/svelte/tests/helpers.js @@ -81,7 +81,11 @@ export async function compile_directory( if (file.endsWith('.js')) { const out = `${output_dir}/${file}`; if (file.endsWith('.svelte.js')) { - const compiled = compileModule(text, opts); + const compiled = compileModule(text, { + filename: opts.filename, + generate: opts.generate, + dev: opts.dev + }); write(out, compiled.js.code); } else { // for non-runes tests, just re-export from the original source file — this diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/Counter.svelte new file mode 100644 index 000000000000..ffe6ef75c4ed --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/Counter.svelte @@ -0,0 +1,7 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/_config.js new file mode 100644 index 000000000000..dc600e1461b5 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/_config.js @@ -0,0 +1,37 @@ +import { test } from '../../test'; + +/** @type {typeof console.warn} */ +let warn; + +/** @type {any[]} */ +let warnings = []; + +export default test({ + html: ``, + + compileOptions: { + dev: true + }, + + before_test: () => { + warn = console.warn; + + console.warn = (...args) => { + warnings.push(...args); + }; + }, + + after_test: () => { + console.warn = warn; + warnings = []; + }, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + await btn?.click(); + + assert.htmlEqual(target.innerHTML, ``); + + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/main.svelte new file mode 100644 index 000000000000..5f1c7461f636 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/main.svelte @@ -0,0 +1,9 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/state.svelte.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/state.svelte.js new file mode 100644 index 000000000000..6881c2faf66b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/state.svelte.js @@ -0,0 +1,3 @@ +export let global = $state({ + object: { count: -1 } +}); From 9eb17cf972c60adce93eb2983d11261585ab3496 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 10:14:30 -0500 Subject: [PATCH 43/46] fix --- .../3-transform/client/transform-client.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 0eac2dc5f0f8..f50069ea7507 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -346,17 +346,22 @@ export function client_component(source, analysis, options) { ) ]; - if (options.dev && options.filename) { - // add `App.filename = 'App.svelte'` so that we can print useful messages later - body.push( - b.stmt( - b.assignment( - '=', - b.member(b.id(analysis.name), b.id('filename')), - b.literal(options.filename) + if (options.dev) { + if (options.filename) { + // add `App.filename = 'App.svelte'` so that we can print useful messages later + body.push( + b.stmt( + b.assignment( + '=', + b.member(b.id(analysis.name), b.id('filename')), + b.literal(options.filename) + ) ) - ) - ); + ); + } + + body.unshift(b.stmt(b.call(b.id('$.mark_module_start'), b.id(analysis.name)))); + body.push(b.stmt(b.call(b.id('$.mark_module_end')))); } if (options.discloseVersion) { @@ -451,11 +456,6 @@ export function client_component(source, analysis, options) { } } - if (options.dev && state.options.filename) { - body.unshift(b.stmt(b.call(b.id('$.mark_module_start'), b.id(analysis.name)))); - body.push(b.stmt(b.call(b.id('$.mark_module_end')))); - } - return { type: 'Program', sourceType: 'module', From 78e796373d4fc26756754bba0a6d147fa4d67009 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 10:19:15 -0500 Subject: [PATCH 44/46] tidy up --- packages/svelte/src/internal/client/proxy.js | 6 +++--- packages/svelte/src/internal/client/runtime.js | 14 +++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 9c8c169ee321..e17edc248d47 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -75,6 +75,8 @@ export function proxy(value, immutable = true, owners) { }); if (DEV) { + // set ownership — either of the parent proxy's owners (if provided) or, + // when calling `$.proxy(...)`, to the current component if such there be // @ts-expect-error value[STATE_SYMBOL].o = owners === undefined @@ -260,9 +262,7 @@ const state_proxy_handler = { set(target, prop, value) { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) { - set(s, proxy(value, metadata.i, metadata.o)); - } + if (s !== undefined) set(s, proxy(value, metadata.i, metadata.o)); const is_array = metadata.a; const not_has = !(prop in target); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 476c2483b479..da08baf1a10e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -934,15 +934,11 @@ export function unsubscribe_on_destroy(stores) { * @returns {V} */ export function get(signal) { - if (DEV) { - const debuggable = /** @type {import('./types.js').SignalDebug} */ (signal); - + // @ts-expect-error + if (DEV && signal.inspect && inspect_fn) { + /** @type {import('./types.js').SignalDebug} */ (signal).inspect.add(inspect_fn); // @ts-expect-error - if (signal.inspect && inspect_fn) { - debuggable.inspect.add(inspect_fn); - // @ts-expect-error - inspect_captured_signals.push(signal); - } + inspect_captured_signals.push(signal); } const flags = signal.f; @@ -1996,7 +1992,7 @@ export function init() { * @param {Set} visited * @returns {void} */ -export function deep_read(value, visited = new Set()) { +function deep_read(value, visited = new Set()) { if (typeof value === 'object' && value !== null && !visited.has(value)) { visited.add(value); for (let key in value) { From f88c32f7388952db5eed4ecc36528171b5299635 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 10:50:25 -0500 Subject: [PATCH 45/46] update filename stuff --- .../phases/3-transform/client/transform-client.js | 13 ++++++++----- packages/svelte/tests/helpers.js | 8 ++++---- .../non-local-mutation-discouraged/_config.js | 2 +- .../bind-this/_expected/client/index.svelte.js | 2 +- .../bind-this/_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/server/index.svelte.js | 2 +- .../hello-world/_expected/client/index.svelte.js | 2 +- .../hello-world/_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 2 +- .../_expected/server/index.svelte.js | 2 +- .../svelte-element/_expected/client/index.svelte.js | 2 +- .../svelte-element/_expected/server/index.svelte.js | 2 +- 17 files changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index f50069ea7507..8e2b5acef2f0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -348,14 +348,17 @@ export function client_component(source, analysis, options) { if (options.dev) { if (options.filename) { + let filename = options.filename; + if (/(\/|\w:)/.test(options.filename)) { + // filename is absolute — truncate it + const parts = filename.split(/[/\\]/); + filename = parts.length > 3 ? ['...', ...parts.slice(-3)].join('/') : filename; + } + // add `App.filename = 'App.svelte'` so that we can print useful messages later body.push( b.stmt( - b.assignment( - '=', - b.member(b.id(analysis.name), b.id('filename')), - b.literal(options.filename) - ) + b.assignment('=', b.member(b.id(analysis.name), b.id('filename')), b.literal(filename)) ) ); } diff --git a/packages/svelte/tests/helpers.js b/packages/svelte/tests/helpers.js index 7b8da31dd28a..16c923391d6b 100644 --- a/packages/svelte/tests/helpers.js +++ b/packages/svelte/tests/helpers.js @@ -73,7 +73,7 @@ export async function compile_directory( let text = fs.readFileSync(`${cwd}/${file}`, 'utf-8'); let opts = { - filename: file, + filename: path.join(cwd, file), ...compileOptions, generate }; @@ -86,7 +86,7 @@ export async function compile_directory( generate: opts.generate, dev: opts.dev }); - write(out, compiled.js.code); + write(out, compiled.js.code.replace(`v${VERSION}`, 'VERSION')); } else { // for non-runes tests, just re-export from the original source file — this // allows the `_config.js` module to import shared state to use in tests @@ -121,8 +121,8 @@ export async function compile_directory( } const compiled = compile(text, { - outputFilename: `_output/${generate}/${file}${file.endsWith('.js') ? '' : '.js'}`, - cssOutputFilename: `_output/${generate}/${file}.css`, + outputFilename: `${output_dir}/${file}${file.endsWith('.js') ? '' : '.js'}`, + cssOutputFilename: `${output_dir}/${file}.css`, ...opts }); compiled.js.code = compiled.js.code.replace(`v${VERSION}`, 'VERSION'); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js index 37b5394f81a3..7a78630da530 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-discouraged/_config.js @@ -41,7 +41,7 @@ export default test({ assert.htmlEqual(target.innerHTML, ``); assert.deepEqual(warnings, [ - 'Counter.svelte mutated a value owned by main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.' + '.../samples/non-local-mutation-discouraged/Counter.svelte mutated a value owned by .../samples/non-local-mutation-discouraged/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.' ]); } }); diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js index 33136e6faa65..0f261e1c81c9 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Index($$anchor, $$props) { +export default function Bind_this($$anchor, $$props) { $.push($$props, false); $.init(); diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js index 838d15f2a5bf..71b688fe3785 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Bind_this($$payload, $$props) { $.push(false); const anchor = $.create_anchor($$payload); diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js index 99d04e98a365..38a74ef9bb87 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Index($$anchor, $$props) { +export default function Class_state_field_constructor_assignment($$anchor, $$props) { $.push($$props, true); class Foo { diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js index 4d99f0e9e717..6ba11ebb1619 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Class_state_field_constructor_assignment($$payload, $$props) { $.push(true); class Foo { diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js index 8026752a3c79..effa734e1d5f 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Index($$anchor, $$props) { +export default function Each_string_template($$anchor, $$props) { $.push($$props, false); $.init(); diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js index 1c23a0c032f1..b23b316e3d7b 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Each_string_template($$payload, $$props) { $.push(false); const anchor = $.create_anchor($$payload); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index db959fdee415..f34a4933a345 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Index($$anchor, $$props) { +export default function Function_prop_no_getter($$anchor, $$props) { $.push($$props, true); let count = $.source(0); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js index 6c0d2e9922dd..f678bb6ad67d 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Function_prop_no_getter($$payload, $$props) { $.push(true); let count = 0; diff --git a/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js index 749e8abd40ba..f57dd8a9fdc0 100644 --- a/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js @@ -5,7 +5,7 @@ import * as $ from "svelte/internal"; var frag = $.template(`

hello world

`); -export default function Index($$anchor, $$props) { +export default function Hello_world($$anchor, $$props) { $.push($$props, false); $.init(); diff --git a/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js index 1e9394f59ab2..7007fe2b48de 100644 --- a/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/hello-world/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Hello_world($$payload, $$props) { $.push(false); $$payload.out += `

hello world

`; $.pop(); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js index 984b485431c9..3da681ea5394 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js @@ -12,7 +12,7 @@ function reset(_, str, tpl) { var frag = $.template(` `, true); -export default function Index($$anchor, $$props) { +export default function State_proxy_literal($$anchor, $$props) { $.push($$props, true); let str = $.source(''); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js index 69e294b76db5..c541299e36a7 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function State_proxy_literal($$payload, $$props) { $.push(true); let str = ''; diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js index bcb630f92867..b2734b8c4648 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js @@ -3,7 +3,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal"; -export default function Index($$anchor, $$props) { +export default function Svelte_element($$anchor, $$props) { $.push($$props, true); let tag = $.prop($$props, "tag", 3, 'hr'); diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js index 51bb86c2b236..405a45227197 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js @@ -2,7 +2,7 @@ // Note: compiler output will change before 5.0 is released! import * as $ from "svelte/internal/server"; -export default function Index($$payload, $$props) { +export default function Svelte_element($$payload, $$props) { $.push(true); let { tag = 'hr' } = $$props; From 26455c352e9d42cf9b33ac27e8ea0bbc867451be Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Feb 2024 11:58:49 -0500 Subject: [PATCH 46/46] changeset --- .changeset/rich-olives-yell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rich-olives-yell.md diff --git a/.changeset/rich-olives-yell.md b/.changeset/rich-olives-yell.md new file mode 100644 index 000000000000..e771b37ef797 --- /dev/null +++ b/.changeset/rich-olives-yell.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: replace proxy-based readonly validation with stack-trace-based ownership tracking