diff --git a/.changeset/sweet-ants-care.md b/.changeset/sweet-ants-care.md new file mode 100644 index 000000000000..b4805626ab4e --- /dev/null +++ b/.changeset/sweet-ants-care.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: rework binding ownership validation diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 284e9a7c3e57..77d1df4cdde2 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -161,7 +161,7 @@ Tried to unmount a component that was not mounted ### ownership_invalid_binding ``` -%parent% passed a value to %child% with `bind:`, but the value is owned by %owner%. Consider creating a binding between %owner% and %parent% +%parent% passed property `%prop%` to %child% with `bind:`, but its parent component %owner% did not declare `%prop%` as a binding. Consider creating a binding between %owner% and %parent% (e.g. `bind:%prop%={...}` instead of `%prop%={...}`) ``` Consider three components `GrandParent`, `Parent` and `Child`. If you do ``, inside `GrandParent` pass on the variable via `` (note the missing `bind:`) and then do `` inside `Parent`, this warning is thrown. @@ -171,11 +171,7 @@ To fix it, `bind:` to the value instead of just passing a property (i.e. in this ### ownership_invalid_mutation ``` -Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead -``` - -``` -%component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead +Mutating unbound props (`%name%`, at %location%) is strongly discouraged. Consider using `bind:%prop%={...}` in %parent% (or using a callback) instead ``` Consider the following code: diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 943cf6f01f4f..f8e9ebd8a047 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -132,7 +132,7 @@ During development, this error is often preceeded by a `console.error` detailing ## ownership_invalid_binding -> %parent% passed a value to %child% with `bind:`, but the value is owned by %owner%. Consider creating a binding between %owner% and %parent% +> %parent% passed property `%prop%` to %child% with `bind:`, but its parent component %owner% did not declare `%prop%` as a binding. Consider creating a binding between %owner% and %parent% (e.g. `bind:%prop%={...}` instead of `%prop%={...}`) Consider three components `GrandParent`, `Parent` and `Child`. If you do ``, inside `GrandParent` pass on the variable via `` (note the missing `bind:`) and then do `` inside `Parent`, this warning is thrown. @@ -140,9 +140,7 @@ To fix it, `bind:` to the value instead of just passing a property (i.e. in this ## ownership_invalid_mutation -> Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead - -> %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead +> Mutating unbound props (`%name%`, at %location%) is strongly discouraged. Consider using `bind:%prop%={...}` in %parent% (or using a callback) instead Consider the following code: diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 1f636c32df6d..c62fb03e8fef 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -432,6 +432,7 @@ export function analyze_component(root, source, options) { uses_component_bindings: false, uses_render_tags: false, needs_context: false, + needs_mutation_validation: false, needs_props: false, event_directive_node: null, uses_event_attributes: false, 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 0bdfbae746d0..38fcee8d6fca 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 @@ -393,6 +393,12 @@ export function client_component(analysis, options) { ); } + if (analysis.needs_mutation_validation) { + component_block.body.unshift( + b.var('$$ownership_validator', b.call('$.create_ownership_validator', b.id('$$props'))) + ); + } + const should_inject_context = dev || analysis.needs_context || diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 28e3fabb1990..a37ecd31cc03 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -1,10 +1,10 @@ -/** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression, Identifier, Pattern, PrivateIdentifier, Statement } from 'estree' */ -/** @import { AST, Binding } from '#compiler' */ +/** @import { ArrowFunctionExpression, AssignmentExpression, Expression, FunctionDeclaration, FunctionExpression, Identifier, Node, Pattern, UpdateExpression } from 'estree' */ +/** @import { Binding } from '#compiler' */ /** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */ /** @import { Analysis } from '../../types.js' */ /** @import { Scope } from '../../scope.js' */ import * as b from '../../../utils/builders.js'; -import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js'; +import { is_simple_expression } from '../../../utils/ast.js'; import { PROPS_IS_LAZY_INITIAL, PROPS_IS_IMMUTABLE, @@ -13,7 +13,8 @@ import { PROPS_IS_BINDABLE } from '../../../../constants.js'; import { dev } from '../../../state.js'; -import { get_value } from './visitors/shared/declarations.js'; +import { walk } from 'zimmerframe'; +import { validate_mutation } from './visitors/shared/utils.js'; /** * @param {Binding} binding @@ -110,6 +111,30 @@ function get_hoisted_params(node, context) { } } } + + if (dev) { + // this is a little hacky, but necessary for ownership validation + // to work inside hoisted event handlers + + /** + * @param {AssignmentExpression | UpdateExpression} node + * @param {{ next: () => void, stop: () => void }} context + */ + function visit(node, { next, stop }) { + if (validate_mutation(node, /** @type {any} */ (context), node) !== node) { + params.push(b.id('$$ownership_validator')); + stop(); + } else { + next(); + } + } + + walk(/** @type {Node} */ (node), null, { + AssignmentExpression: visit, + UpdateExpression: visit + }); + } + return params; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 850cd83b15b6..4baa1c8e6ce5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -7,9 +7,10 @@ import { get_attribute_expression, is_event_attribute } from '../../../../utils/ast.js'; -import { dev, is_ignored, locate_node } from '../../../../state.js'; +import { dev, locate_node } from '../../../../state.js'; import { should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; +import { validate_mutation } from './shared/utils.js'; /** * @param {AssignmentExpression} node @@ -20,9 +21,7 @@ export function AssignmentExpression(node, context) { visit_assignment_expression(node, context, build_assignment) ?? context.next() ); - return is_ignored(node, 'ownership_invalid_mutation') - ? b.call('$.skip_ownership_validation', b.thunk(expression)) - : expression; + return validate_mutation(node, context, expression); } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 24c20d7f947f..efc3c95c3c34 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -161,33 +161,6 @@ export function ClassBody(node, context) { body.push(/** @type {MethodDefinition} **/ (context.visit(definition, child_state))); } - if (dev && public_state.size > 0) { - // add an `[$.ADD_OWNER]` method so that a class with state fields can widen ownership - body.push( - b.method( - 'method', - b.id('$.ADD_OWNER'), - [b.id('owner')], - [ - b.stmt( - b.call( - '$.add_owner_to_class', - b.this, - b.id('owner'), - b.array( - Array.from(public_state).map(([name]) => - b.thunk(b.call('$.get', b.member(b.this, b.private_id(name)))) - ) - ), - is_ignored(node, 'ownership_invalid_binding') && b.true - ) - ) - ], - true - ) - ); - } - return { ...node, body }; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index 13c1b4bc51e1..63c03b0eb6f2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -1,8 +1,8 @@ /** @import { AssignmentExpression, Expression, UpdateExpression } from 'estree' */ /** @import { Context } from '../types' */ -import { is_ignored } from '../../../../state.js'; import { object } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; +import { validate_mutation } from './shared/utils.js'; /** * @param {UpdateExpression} node @@ -51,7 +51,5 @@ export function UpdateExpression(node, context) { ); } - return is_ignored(node, 'ownership_invalid_mutation') - ? b.call('$.skip_ownership_validation', b.thunk(update)) - : update; + return validate_mutation(node, context, update); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 2bae4486dc58..2ea68e206e2e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -179,19 +179,29 @@ export function build_component(node, component_name, context, anchor = context. } else if (attribute.type === 'BindDirective') { const expression = /** @type {Expression} */ (context.visit(attribute.expression)); - if (dev && attribute.name !== 'this') { - binding_initializers.push( - b.stmt( - b.call( - b.id('$.add_owner_effect'), - expression.type === 'SequenceExpression' - ? expression.expressions[0] - : b.thunk(expression), - b.id(component_name), - is_ignored(node, 'ownership_invalid_binding') && b.true + if ( + dev && + attribute.name !== 'this' && + !is_ignored(node, 'ownership_invalid_binding') && + // bind:x={() => x.y, y => x.y = y} will be handled by the assignment expression binding validation + attribute.expression.type !== 'SequenceExpression' + ) { + const left = object(attribute.expression); + const binding = left && context.state.scope.get(left.name); + + if (binding?.kind === 'bindable_prop' || binding?.kind === 'prop') { + context.state.analysis.needs_mutation_validation = true; + binding_initializers.push( + b.stmt( + b.call( + '$$ownership_validator.binding', + b.literal(binding.node.name), + b.id(component_name), + b.thunk(expression) + ) ) - ) - ); + ); + } } if (expression.type === 'SequenceExpression') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index df6308d6316a..af6e56f70c81 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -1,13 +1,13 @@ -/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */ +/** @import { AssignmentExpression, Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Literal, Super, UpdateExpression } from 'estree' */ /** @import { AST, ExpressionMetadata } from '#compiler' */ -/** @import { ComponentClientTransformState } from '../../types' */ +/** @import { ComponentClientTransformState, Context } from '../../types' */ import { walk } from 'zimmerframe'; import { object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js'; import { regex_is_valid_identifier } from '../../../../patterns.js'; import is_reference from 'is-reference'; -import { locator } from '../../../../../state.js'; +import { dev, is_ignored, locator } from '../../../../../state.js'; import { create_derived } from '../../utils.js'; /** @@ -295,3 +295,60 @@ export function validate_binding(state, binding, expression) { ) ); } + +/** + * In dev mode validate mutations to props + * @param {AssignmentExpression | UpdateExpression} node + * @param {Context} context + * @param {Expression} expression + */ +export function validate_mutation(node, context, expression) { + let left = /** @type {Expression | Super} */ ( + node.type === 'AssignmentExpression' ? node.left : node.argument + ); + + if (!dev || left.type !== 'MemberExpression' || is_ignored(node, 'ownership_invalid_mutation')) { + return expression; + } + + const name = object(left); + if (!name) return expression; + + const binding = context.state.scope.get(name.name); + if (binding?.kind !== 'prop' && binding?.kind !== 'bindable_prop') return expression; + + const state = /** @type {ComponentClientTransformState} */ (context.state); + state.analysis.needs_mutation_validation = true; + + /** @type {Array} */ + const path = []; + + while (left.type === 'MemberExpression') { + if (left.property.type === 'Literal') { + path.unshift(left.property); + } else if (left.property.type === 'Identifier') { + if (left.computed) { + path.unshift(left.property); + } else { + path.unshift(b.literal(left.property.name)); + } + } else { + return expression; + } + + left = left.object; + } + + path.unshift(b.literal(name.name)); + + const loc = locator(/** @type {number} */ (left.start)); + + return b.call( + '$$ownership_validator.mutation', + b.literal(binding.prop_alias), + b.array(path), + expression, + loc && b.literal(loc.line), + loc && b.literal(loc.column) + ); +} diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index abe2b115de02..f09b88130305 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -53,6 +53,7 @@ export interface ComponentAnalysis extends Analysis { uses_component_bindings: boolean; uses_render_tags: boolean; needs_context: boolean; + needs_mutation_validation: boolean; needs_props: boolean; /** Set to the first event directive (on:x) found on a DOM element in the code */ event_directive_node: AST.OnDirective | null; diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 21377c1cc85f..7e5196c606b4 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -23,6 +23,5 @@ export const EFFECT_HAS_DERIVED = 1 << 20; export const EFFECT_IS_UPDATING = 1 << 21; export const STATE_SYMBOL = Symbol('$state'); -export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); export const LEGACY_PROPS = Symbol('legacy props'); export const LOADING_ATTR_SYMBOL = Symbol(''); diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index bfca9d5e6a72..7a2fdd0edb6d 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -1,7 +1,6 @@ /** @import { ComponentContext } from '#client' */ import { DEV } from 'esm-env'; -import { add_owner } from './dev/ownership.js'; import { lifecycle_outside_component } from '../shared/errors.js'; import { source } from './reactivity/sources.js'; import { @@ -67,15 +66,6 @@ export function getContext(key) { */ export function setContext(key, context) { const context_map = get_or_init_context_map('setContext'); - - if (DEV) { - // When state is put into context, we treat as if it's global from now on. - // We do for performance reasons (it's for example very expensive to call - // getContext on a big object many times when part of a list component) - // and danger of false positives. - untrack(() => add_owner(context, null, true)); - } - context_map.set(key, context); return context; } diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 62119b36dbd6..6c40a744dfc5 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,12 +1,11 @@ -/** @import { ProxyMetadata } from '#client' */ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { STATE_SYMBOL_METADATA } from '../constants.js'; -import { render_effect, user_pre_effect } from '../reactivity/effects.js'; -import { dev_current_component_function } from '../context.js'; -import { get_prototype_of } from '../../shared/utils.js'; +import { get_descriptor } from '../../shared/utils.js'; +import { LEGACY_PROPS, STATE_SYMBOL } from '../constants.js'; +import { FILENAME } from '../../../constants.js'; +import { component_context } from '../context.js'; import * as w from '../warnings.js'; -import { FILENAME, UNINITIALIZED } from '../../../constants.js'; +import { sanitize_location } from '../../../utils.js'; /** @type {Record>} */ const boundaries = {}; @@ -71,8 +70,6 @@ export function get_component() { return null; } -export const ADD_OWNER = Symbol('ADD_OWNER'); - /** * 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 @@ -108,197 +105,69 @@ export function mark_module_end(component) { } /** - * @param {any} object - * @param {any | null} owner - * @param {boolean} [global] - * @param {boolean} [skip_warning] + * Sets up a validator that + * - traverses the path of a prop to find out if it is allowed to be mutated + * - checks that the binding chain is not interrupted + * @param {Record} props */ -export function add_owner(object, owner, global = false, skip_warning = false) { - if (object && !global) { - const component = dev_current_component_function; - const metadata = object[STATE_SYMBOL_METADATA]; - if (metadata && !has_owner(metadata, component)) { - let original = get_owner(metadata); - - if (owner && owner[FILENAME] !== component[FILENAME] && !skip_warning) { - w.ownership_invalid_binding(component[FILENAME], owner[FILENAME], original[FILENAME]); +export function create_ownership_validator(props) { + const component = component_context?.function; + const parent = component_context?.p?.function; + + return { + /** + * @param {string} prop + * @param {any[]} path + * @param {any} result + * @param {number} line + * @param {number} column + */ + mutation: (prop, path, result, line, column) => { + const name = path[0]; + if (is_bound(props, name) || !parent) { + return result; } - } - } - add_owner_to_object(object, owner, new Set()); -} - -/** - * @param {() => unknown} get_object - * @param {any} Component - * @param {boolean} [skip_warning] - */ -export function add_owner_effect(get_object, Component, skip_warning = false) { - user_pre_effect(() => { - add_owner(get_object(), Component, false, skip_warning); - }); -} - -/** - * @param {any} _this - * @param {Function} owner - * @param {Array<() => any>} getters - * @param {boolean} skip_warning - */ -export function add_owner_to_class(_this, owner, getters, skip_warning) { - _this[ADD_OWNER].current ||= getters.map(() => UNINITIALIZED); - - for (let i = 0; i < getters.length; i += 1) { - const current = getters[i](); - // For performance reasons we only re-add the owner if the state has changed - if (current !== _this[ADD_OWNER][i]) { - _this[ADD_OWNER].current[i] = current; - add_owner(current, owner, false, skip_warning); - } - } -} - -/** - * @param {ProxyMetadata | null} from - * @param {ProxyMetadata} to - */ -export function widen_ownership(from, to) { - if (to.owners === null) { - return; - } - - while (from) { - if (from.owners === null) { - to.owners = null; - break; - } - - for (const owner of from.owners) { - to.owners.add(owner); - } - - from = from.parent; - } -} - -/** - * @param {any} object - * @param {Function | null} owner If `null`, then the object is globally owned and will not be checked - * @param {Set} seen - */ -function add_owner_to_object(object, owner, seen) { - const metadata = /** @type {ProxyMetadata} */ (object?.[STATE_SYMBOL_METADATA]); + let value = props[name]; - if (metadata) { - // this is a state proxy, add owner directly, if not globally shared - if ('owners' in metadata && metadata.owners != null) { - if (owner) { - metadata.owners.add(owner); - } else { - metadata.owners = null; + for (let i = 1; i < path.length - 1; i++) { + if (!value?.[STATE_SYMBOL]) { + return result; + } + value = value[path[i]]; } - } - } else if (object && typeof object === 'object') { - if (seen.has(object)) return; - seen.add(object); - if (ADD_OWNER in object && object[ADD_OWNER]) { - // this is a class with state fields. we put this in a render effect - // so that if state is replaced (e.g. `instance.name = { first, last }`) - // the new state is also co-owned by the caller of `getContext` - render_effect(() => { - object[ADD_OWNER](owner); - }); - } else { - var proto = get_prototype_of(object); - if (proto === Object.prototype) { - // recurse until we find a state proxy - for (const key in object) { - if (Object.getOwnPropertyDescriptor(object, key)?.get) { - // Similar to the class case; the getter could update with a new state - let current = UNINITIALIZED; - render_effect(() => { - const next = object[key]; - if (current !== next) { - current = next; - add_owner_to_object(next, owner, seen); - } - }); - } else { - add_owner_to_object(object[key], owner, seen); - } - } - } else if (proto === Array.prototype) { - // recurse until we find a state proxy - for (let i = 0; i < object.length; i += 1) { - add_owner_to_object(object[i], owner, seen); - } + const location = sanitize_location(`${component[FILENAME]}:${line}:${column}`); + + w.ownership_invalid_mutation(name, location, prop, parent[FILENAME]); + + return result; + }, + /** + * @param {any} key + * @param {any} child_component + * @param {() => any} value + */ + binding: (key, child_component, value) => { + if (!is_bound(props, key) && parent && value()?.[STATE_SYMBOL]) { + w.ownership_invalid_binding( + component[FILENAME], + key, + child_component[FILENAME], + parent[FILENAME] + ); } } - } + }; } /** - * @param {ProxyMetadata} metadata - * @param {Function} component - * @returns {boolean} + * @param {Record} props + * @param {string} prop_name */ -function has_owner(metadata, component) { - if (metadata.owners === null) { - return true; - } - - return ( - metadata.owners.has(component) || - // This helps avoid false positives when using HMR, where the component function is replaced - (FILENAME in component && - [...metadata.owners].some( - (owner) => /** @type {any} */ (owner)[FILENAME] === component[FILENAME] - )) || - (metadata.parent !== null && has_owner(metadata.parent, component)) - ); -} - -/** - * @param {ProxyMetadata} metadata - * @returns {any} - */ -function get_owner(metadata) { - return ( - metadata?.owners?.values().next().value ?? - get_owner(/** @type {ProxyMetadata} */ (metadata.parent)) - ); -} - -let skip = false; - -/** - * @param {() => any} fn - */ -export function skip_ownership_validation(fn) { - skip = true; - fn(); - skip = false; -} - -/** - * @param {ProxyMetadata} metadata - */ -export function check_ownership(metadata) { - if (skip) return; - - const component = get_component(); - - if (component && !has_owner(metadata, component)) { - let original = get_owner(metadata); - - // @ts-expect-error - if (original[FILENAME] !== component[FILENAME]) { - // @ts-expect-error - w.ownership_invalid_mutation(component[FILENAME], original[FILENAME]); - } else { - w.ownership_invalid_mutation(); - } - } +function is_bound(props, prop_name) { + // Can be the case when someone does `mount(Component, props)` with `let props = $state({...})` + // or `createClassComponent(Component, props)` + const is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props; + return !!get_descriptor(props, prop_name)?.set || (is_entry_props && prop_name in props); } diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index a5f93e8b171b..7eed8a744afa 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -4,15 +4,7 @@ export { assign, assign_and, assign_or, assign_nullish } from './dev/assign.js'; export { cleanup_styles } from './dev/css.js'; export { add_locations } from './dev/elements.js'; export { hmr } from './dev/hmr.js'; -export { - ADD_OWNER, - add_owner, - mark_module_start, - mark_module_end, - add_owner_effect, - add_owner_to_class, - skip_ownership_validation -} from './dev/ownership.js'; +export { mark_module_start, mark_module_end, create_ownership_validator } from './dev/ownership.js'; export { check_target, legacy_api } from './dev/legacy.js'; export { trace } from './dev/tracing.js'; export { inspect } from './dev/inspect.js'; diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index fab271c91652..5e0aa3dbc35f 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -1,7 +1,6 @@ -/** @import { ProxyMetadata, Source } from '#client' */ +/** @import { Source } from '#client' */ import { DEV } from 'esm-env'; import { get, active_effect, active_reaction, set_active_reaction } from './runtime.js'; -import { component_context } from './context.js'; import { array_prototype, get_descriptor, @@ -9,24 +8,19 @@ import { is_array, object_prototype } from '../shared/utils.js'; -import { check_ownership, widen_ownership } from './dev/ownership.js'; import { state as source, set } from './reactivity/sources.js'; -import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js'; +import { STATE_SYMBOL } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; import { get_stack } from './dev/tracing.js'; import { tracing_mode_flag } from '../flags/index.js'; -/** @type {ProxyMetadata | null} */ -var parent_metadata = null; - /** * @template T * @param {T} value - * @param {Source} [prev] dev mode only * @returns {T} */ -export function proxy(value, prev) { +export function proxy(value) { // if non-proxyable, or is already a proxy, return `value` if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) { return value; @@ -55,16 +49,7 @@ export function proxy(value, prev) { set_active_reaction(reaction); /** @type {T} */ - var result; - - if (DEV) { - var previous_metadata = parent_metadata; - parent_metadata = metadata; - result = fn(); - parent_metadata = previous_metadata; - } else { - result = fn(); - } + var result = fn(); set_active_reaction(previous_reaction); return result; @@ -76,31 +61,6 @@ export function proxy(value, prev) { sources.set('length', source(/** @type {any[]} */ (value).length, stack)); } - /** @type {ProxyMetadata} */ - var metadata; - - if (DEV) { - metadata = { - parent: parent_metadata, - owners: null - }; - - if (prev) { - // Reuse owners from previous state; necessary because reassignment is not guaranteed to have correct component context. - // If no previous proxy exists we play it safe and assume ownerless state - // @ts-expect-error - const prev_owners = prev.v?.[STATE_SYMBOL_METADATA]?.owners; - metadata.owners = prev_owners ? new Set(prev_owners) : null; - } else { - metadata.owners = - parent_metadata === null - ? component_context !== null - ? new Set([component_context.function]) - : null - : new Set(); - } - } - return new Proxy(/** @type {any} */ (value), { defineProperty(_, prop, descriptor) { if ( @@ -160,10 +120,6 @@ export function proxy(value, prev) { }, get(target, prop, receiver) { - if (DEV && prop === STATE_SYMBOL_METADATA) { - return metadata; - } - if (prop === STATE_SYMBOL) { return value; } @@ -179,22 +135,6 @@ export function proxy(value, prev) { if (s !== undefined) { var v = get(s); - - // In case of something like `foo = bar.map(...)`, foo would have ownership - // of the array itself, while the individual items would have ownership - // of the component that created bar. That means if we later do `foo[0].baz = 42`, - // we could get a false-positive ownership violation, since the two proxies - // are not connected to each other via the parent metadata relationship. - // For this reason, we need to widen the ownership of the children - // upon access when we detect they are not connected. - if (DEV) { - /** @type {ProxyMetadata | undefined} */ - var prop_metadata = v?.[STATE_SYMBOL_METADATA]; - if (prop_metadata && prop_metadata?.parent !== metadata) { - widen_ownership(metadata, prop_metadata); - } - } - return v === UNINITIALIZED ? undefined : v; } @@ -225,10 +165,6 @@ export function proxy(value, prev) { }, has(target, prop) { - if (DEV && prop === STATE_SYMBOL_METADATA) { - return true; - } - if (prop === STATE_SYMBOL) { return true; } @@ -295,15 +231,6 @@ export function proxy(value, prev) { ); } - if (DEV) { - /** @type {ProxyMetadata | undefined} */ - var prop_metadata = value?.[STATE_SYMBOL_METADATA]; - if (prop_metadata && prop_metadata?.parent !== metadata) { - widen_ownership(metadata, prop_metadata); - } - check_ownership(metadata); - } - var descriptor = Reflect.getOwnPropertyDescriptor(target, prop); // Set the new value before updating any signals so that any listeners get the new value diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index e4834902fe3f..aa65d5354ac7 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -139,7 +139,7 @@ export function set(source, value, should_proxy = false) { e.state_unsafe_mutation(); } - let new_value = should_proxy ? proxy(value, source) : value; + let new_value = should_proxy ? proxy(value) : value; return internal_set(source, new_value); } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 0c260a0a9ffa..b46bdf201341 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -179,14 +179,6 @@ export type TaskCallback = (now: number) => boolean | void; export type TaskEntry = { c: TaskCallback; f: () => void }; -/** Dev-only */ -export interface ProxyMetadata { - /** The components that 'own' this state, if any. `null` means no owners, i.e. everyone can mutate this state. */ - owners: null | Set; - /** The parent metadata object */ - parent: null | ProxyMetadata; -} - export type ProxyStateObject> = T & { [STATE_SYMBOL]: T; }; diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 250c6eca2fe9..c84b487e280d 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -129,27 +129,30 @@ export function lifecycle_double_unmount() { } /** - * %parent% passed a value to %child% with `bind:`, but the value is owned by %owner%. Consider creating a binding between %owner% and %parent% + * %parent% passed property `%prop%` to %child% with `bind:`, but its parent component %owner% did not declare `%prop%` as a binding. Consider creating a binding between %owner% and %parent% (e.g. `bind:%prop%={...}` instead of `%prop%={...}`) * @param {string} parent + * @param {string} prop * @param {string} child * @param {string} owner */ -export function ownership_invalid_binding(parent, child, owner) { +export function ownership_invalid_binding(parent, prop, child, owner) { if (DEV) { - console.warn(`%c[svelte] ownership_invalid_binding\n%c${parent} passed a value to ${child} with \`bind:\`, but the value is owned by ${owner}. Consider creating a binding between ${owner} and ${parent}\nhttps://svelte.dev/e/ownership_invalid_binding`, bold, normal); + console.warn(`%c[svelte] ownership_invalid_binding\n%c${parent} passed property \`${prop}\` to ${child} with \`bind:\`, but its parent component ${owner} did not declare \`${prop}\` as a binding. Consider creating a binding between ${owner} and ${parent} (e.g. \`bind:${prop}={...}\` instead of \`${prop}={...}\`)\nhttps://svelte.dev/e/ownership_invalid_binding`, bold, normal); } else { console.warn(`https://svelte.dev/e/ownership_invalid_binding`); } } /** - * %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead - * @param {string | undefined | null} [component] - * @param {string | undefined | null} [owner] + * Mutating unbound props (`%name%`, at %location%) is strongly discouraged. Consider using `bind:%prop%={...}` in %parent% (or using a callback) instead + * @param {string} name + * @param {string} location + * @param {string} prop + * @param {string} parent */ -export function ownership_invalid_mutation(component, owner) { +export function ownership_invalid_mutation(name, location, prop, parent) { if (DEV) { - console.warn(`%c[svelte] ownership_invalid_mutation\n%c${component ? `${component} mutated a value owned by ${owner}. This is strongly discouraged. Consider passing values to child components with \`bind:\`, or use a callback instead` : 'Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'}\nhttps://svelte.dev/e/ownership_invalid_mutation`, bold, normal); + console.warn(`%c[svelte] ownership_invalid_mutation\n%cMutating unbound props (\`${name}\`, at ${location}) is strongly discouraged. Consider using \`bind:${prop}={...}\` in ${parent} (or using a callback) instead\nhttps://svelte.dev/e/ownership_invalid_mutation`, bold, normal); } else { console.warn(`https://svelte.dev/e/ownership_invalid_mutation`); } diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index d4d106d56deb..ada318e85ac7 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -465,8 +465,10 @@ export function is_raw_text_element(name) { /** * Prevent devtools trying to make `location` a clickable link by inserting a zero-width space - * @param {string | undefined} location + * @template {string | undefined} T + * @param {T} location + * @returns {T}; */ export function sanitize_location(location) { - return location?.replace(/\//g, '/\u200b'); + return /** @type {T} */ (location?.replace(/\//g, '/\u200b')); } 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 62c696124285..84526610260d 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 @@ -10,7 +10,7 @@ export default test({ test({ assert, target, warnings }) { const warning = - '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'; + 'Mutating unbound props (`object`, at Counter.svelte:5:23) is strongly discouraged. Consider using `bind:object={...}` in main.svelte (or using a callback) instead'; const [btn1, btn2] = target.querySelectorAll('button'); btn1.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/_config.js deleted file mode 100644 index b4864154c39a..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/_config.js +++ /dev/null @@ -1,11 +0,0 @@ -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - - async test({ assert, warnings }) { - assert.deepEqual(warnings, []); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/child.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/child.svelte deleted file mode 100644 index 13de75364752..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/child.svelte +++ /dev/null @@ -1,18 +0,0 @@ - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/main.svelte deleted file mode 100644 index 8a6922e9e250..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-global-2/main.svelte +++ /dev/null @@ -1,5 +0,0 @@ - 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-1/_config.js similarity index 74% rename from packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/_config.js rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/_config.js index c07b9ce129dc..96b18d1854c8 100644 --- 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-1/_config.js @@ -8,7 +8,7 @@ let warn; let warnings = []; export default test({ - html: ``, + html: ``, compileOptions: { dev: true @@ -34,8 +34,8 @@ export default test({ btn?.click(); }); - assert.htmlEqual(target.innerHTML, ``); + assert.htmlEqual(target.innerHTML, ``); - assert.deepEqual(warnings, []); + assert.deepEqual(warnings, [], 'expected getContext to have widened ownership'); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/main.svelte new file mode 100644 index 000000000000..2dd7cab141d6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/main.svelte @@ -0,0 +1,9 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/sub.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/sub.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/sub.svelte rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-1/sub.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/_config.js index c07b9ce129dc..66f1726a2aef 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/_config.js @@ -1,41 +1,24 @@ import { flushSync } from 'svelte'; 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); - }; - }, + test({ assert, target, warnings }) { + const [btn1, btn2] = target.querySelectorAll('button'); - after_test: () => { - console.warn = warn; - warnings = []; - }, + flushSync(() => { + btn1.click(); + }); - test({ assert, target }) { - const btn = target.querySelector('button'); + assert.deepEqual(warnings.length, 0); flushSync(() => { - btn?.click(); + btn2.click(); }); - assert.htmlEqual(target.innerHTML, ``); - - assert.deepEqual(warnings, []); + assert.deepEqual(warnings.length, 1); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/main.svelte index ad450a937e40..0be7e434e475 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/main.svelte @@ -1,9 +1,8 @@ - - + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/state.svelte.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/state.svelte.js index 3e7a68cf97d8..2906b9bce52b 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/state.svelte.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/state.svelte.js @@ -1 +1,14 @@ -export let global = $state({}); +export function create_my_state() { + const my_state = $state({ + a: 0 + }); + + function inc() { + my_state.a++; + } + + return { + my_state, + inc + }; +} diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/sub.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/sub.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/sub.svelte rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-2/sub.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/Child.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/Child.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/Child.svelte rename to packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/Child.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/_config.js index 96b18d1854c8..ab7327ab8b82 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/_config.js @@ -1,41 +1,24 @@ import { flushSync } from 'svelte'; 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); - }; - }, + async test({ assert, target, warnings }) { + const [btn1, btn2] = target.querySelectorAll('button'); - after_test: () => { - console.warn = warn; - warnings = []; - }, + flushSync(() => { + btn1.click(); + }); - test({ assert, target }) { - const btn = target.querySelector('button'); + assert.deepEqual(warnings.length, 0); flushSync(() => { - btn?.click(); + btn2.click(); }); - assert.htmlEqual(target.innerHTML, ``); - - assert.deepEqual(warnings, [], 'expected getContext to have widened ownership'); + assert.deepEqual(warnings.length, 1); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/main.svelte index 2dd7cab141d6..8e8343790b1b 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-3/main.svelte @@ -1,9 +1,13 @@ - + + diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/_config.js deleted file mode 100644 index aeb3740dfe71..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/_config.js +++ /dev/null @@ -1,37 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -/** @type {typeof console.warn} */ -let warn; - -/** @type {any[]} */ -let warnings = []; - -export default test({ - compileOptions: { - dev: true - }, - - before_test: () => { - warn = console.warn; - - console.warn = (...args) => { - warnings.push(...args); - }; - }, - - after_test: () => { - console.warn = warn; - warnings = []; - }, - - test({ assert, target }) { - const btn = target.querySelector('button'); - - flushSync(() => { - btn?.click(); - }); - - assert.deepEqual(warnings, []); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/main.svelte deleted file mode 100644 index 2d40c13949a6..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/main.svelte +++ /dev/null @@ -1,11 +0,0 @@ - - - - - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/state.svelte.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/state.svelte.js deleted file mode 100644 index 40790591712c..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/state.svelte.js +++ /dev/null @@ -1,13 +0,0 @@ -class Global { - state = $state({}); - - add_a(a) { - this.state.a = a; - } - - increment_a_b() { - this.state.a.b++; - } -} - -export const global = new Global(); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/sub.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/sub.svelte deleted file mode 100644 index 044904aa187e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-4/sub.svelte +++ /dev/null @@ -1,8 +0,0 @@ - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/_config.js deleted file mode 100644 index 66f1726a2aef..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/_config.js +++ /dev/null @@ -1,24 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - - test({ assert, target, warnings }) { - const [btn1, btn2] = target.querySelectorAll('button'); - - flushSync(() => { - btn1.click(); - }); - - assert.deepEqual(warnings.length, 0); - - flushSync(() => { - btn2.click(); - }); - - assert.deepEqual(warnings.length, 1); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/main.svelte deleted file mode 100644 index 0be7e434e475..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/main.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/state.svelte.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/state.svelte.js deleted file mode 100644 index 2906b9bce52b..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-5/state.svelte.js +++ /dev/null @@ -1,14 +0,0 @@ -export function create_my_state() { - const my_state = $state({ - a: 0 - }); - - function inc() { - my_state.a++; - } - - return { - my_state, - inc - }; -} diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/Child.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/Child.svelte deleted file mode 100644 index aa31fd7606dd..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/Child.svelte +++ /dev/null @@ -1,9 +0,0 @@ - - - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/_config.js deleted file mode 100644 index cc9ea715f0aa..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/_config.js +++ /dev/null @@ -1,37 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -/** @type {typeof console.warn} */ -let warn; - -/** @type {any[]} */ -let warnings = []; - -export default test({ - 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'); - - flushSync(() => { - btn?.click(); - }); - - assert.deepEqual(warnings.length, 0); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/main.svelte deleted file mode 100644 index 92d7dbd2db6c..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/main.svelte +++ /dev/null @@ -1,17 +0,0 @@ - - - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/_config.js deleted file mode 100644 index ab7327ab8b82..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/_config.js +++ /dev/null @@ -1,24 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - - async test({ assert, target, warnings }) { - const [btn1, btn2] = target.querySelectorAll('button'); - - flushSync(() => { - btn1.click(); - }); - - assert.deepEqual(warnings.length, 0); - - flushSync(() => { - btn2.click(); - }); - - assert.deepEqual(warnings.length, 1); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/main.svelte deleted file mode 100644 index 8e8343790b1b..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-7/main.svelte +++ /dev/null @@ -1,13 +0,0 @@ - - - - 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 deleted file mode 100644 index ffe6ef75c4ed..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/Counter.svelte +++ /dev/null @@ -1,7 +0,0 @@ - - - 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 deleted file mode 100644 index 5f1c7461f636..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/main.svelte +++ /dev/null @@ -1,9 +0,0 @@ - - - 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 deleted file mode 100644 index 6881c2faf66b..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner/state.svelte.js +++ /dev/null @@ -1,3 +0,0 @@ -export let global = $state({ - object: { count: -1 } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-2/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-2/_config.js index 87474a05cc33..39fa80c55a4e 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-2/_config.js @@ -8,6 +8,6 @@ export default test({ }, warnings: [ - 'Intermediate.svelte passed a value to Counter.svelte with `bind:`, but the value is owned by main.svelte. Consider creating a binding between main.svelte and Intermediate.svelte' + 'Intermediate.svelte passed property `object` to Counter.svelte with `bind:`, but its parent component main.svelte did not declare `object` as a binding. Consider creating a binding between main.svelte and Intermediate.svelte (e.g. `bind:object={...}` instead of `object={...}`)' ] }); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-3/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-3/_config.js index 66e51843808e..7b8cc676d528 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-3/_config.js @@ -33,7 +33,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' + 'Mutating unbound props (`notshared`, at Counter.svelte:10:23) is strongly discouraged. Consider using `bind:notshared={...}` in main.svelte (or using a callback) instead' ]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-7/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-7/_config.js index e766a946d0dc..bd2ecc28b6f7 100644 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-7/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-7/_config.js @@ -1,7 +1,6 @@ import { flushSync } from 'svelte'; import { ok, test } from '../../test'; -// Tests that proxies widen ownership correctly even if not directly connected to each other export default test({ compileOptions: { dev: true diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterBinding.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterBinding.svelte deleted file mode 100644 index d6da559fb176..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterBinding.svelte +++ /dev/null @@ -1,7 +0,0 @@ - - -

Binding

- - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterContext.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterContext.svelte deleted file mode 100644 index b935f0a472dc..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/CounterContext.svelte +++ /dev/null @@ -1,13 +0,0 @@ - - -

Context

- - diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/_config.js b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/_config.js deleted file mode 100644 index d6d12d01cd09..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/_config.js +++ /dev/null @@ -1,34 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -// Tests that ownership is widened with $derived (on class or on its own) that contains $state -export default test({ - compileOptions: { - dev: true - }, - - test({ assert, target, warnings }) { - const [root, counter_context1, counter_context2, counter_binding1, counter_binding2] = - target.querySelectorAll('button'); - - counter_context1.click(); - counter_context2.click(); - counter_binding1.click(); - counter_binding2.click(); - flushSync(); - - assert.equal(warnings.length, 0); - - root.click(); - flushSync(); - counter_context1.click(); - counter_context2.click(); - counter_binding1.click(); - counter_binding2.click(); - flushSync(); - - assert.equal(warnings.length, 0); - }, - - warnings: [] -}); diff --git a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/main.svelte b/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/main.svelte deleted file mode 100644 index aaade26e162c..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/main.svelte +++ /dev/null @@ -1,46 +0,0 @@ - - -

Parent

- - - -