fix: prevent ownership validation from infering with component context (#11438)

Ownership validation had a false positive when rendering a component as slotted content of another component. To fix this, #11401 did set the current component context to the context the snippet was declared in, not to the context it is rendered in. This was flawed because it means that component context was altered in a way that setContext/getContext failed because the parent chain was incorrect. This fixes that by introducing a separate global (dev time only) which tracks the component function the ownership needs.

fixes #11429
pull/11439/head
Simon H 8 months ago committed by GitHub
parent dfb30aaddd
commit 6e5ab2e678
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: prevent ownership validation from infering with component context

@ -746,12 +746,7 @@ function serialize_inline_component(node, component_name, context) {
if (context.state.options.dev) { if (context.state.options.dev) {
binding_initializers.push( binding_initializers.push(
b.stmt( b.stmt(b.call(b.id('$.add_owner_effect'), b.thunk(expression), b.id(component_name)))
b.call(
b.id('$.user_pre_effect'),
b.thunk(b.call(b.id('$.add_owner'), expression, b.id(component_name)))
)
)
); );
} }

@ -1,8 +1,8 @@
/** @typedef {{ file: string, line: number, column: number }} Location */ /** @typedef {{ file: string, line: number, column: number }} Location */
import { STATE_SYMBOL } from '../constants.js'; import { STATE_SYMBOL } from '../constants.js';
import { render_effect } from '../reactivity/effects.js'; import { render_effect, user_pre_effect } from '../reactivity/effects.js';
import { current_component_context, untrack } from '../runtime.js'; import { dev_current_component_function, set_dev_current_component_function } from '../runtime.js';
import { get_prototype_of } from '../utils.js'; import { get_prototype_of } from '../utils.js';
import * as w from '../warnings.js'; import * as w from '../warnings.js';
@ -109,8 +109,7 @@ export function mark_module_end(component) {
*/ */
export function add_owner(object, owner, global = false) { export function add_owner(object, owner, global = false) {
if (object && !global) { if (object && !global) {
// @ts-expect-error const component = dev_current_component_function;
const component = current_component_context.function;
const metadata = object[STATE_SYMBOL]; const metadata = object[STATE_SYMBOL];
if (metadata && !has_owner(metadata, component)) { if (metadata && !has_owner(metadata, component)) {
let original = get_owner(metadata); let original = get_owner(metadata);
@ -124,6 +123,20 @@ export function add_owner(object, owner, global = false) {
add_owner_to_object(object, owner, new Set()); add_owner_to_object(object, owner, new Set());
} }
/**
* @param {() => unknown} get_object
* @param {any} Component
*/
export function add_owner_effect(get_object, Component) {
var component = dev_current_component_function;
user_pre_effect(() => {
var prev = dev_current_component_function;
set_dev_current_component_function(component);
add_owner(get_object(), Component);
set_dev_current_component_function(prev);
});
}
/** /**
* @param {import('#client').ProxyMetadata<any> | null} from * @param {import('#client').ProxyMetadata<any> | null} from
* @param {import('#client').ProxyMetadata<any>} to * @param {import('#client').ProxyMetadata<any>} to

@ -1,7 +1,11 @@
import { add_snippet_symbol } from '../../../shared/validate.js'; import { add_snippet_symbol } from '../../../shared/validate.js';
import { EFFECT_TRANSPARENT } from '../../constants.js'; import { EFFECT_TRANSPARENT } from '../../constants.js';
import { branch, block, destroy_effect } from '../../reactivity/effects.js'; import { branch, block, destroy_effect } from '../../reactivity/effects.js';
import { current_component_context, set_current_component_context } from '../../runtime.js'; import {
current_component_context,
dev_current_component_function,
set_dev_current_component_function
} from '../../runtime.js';
/** /**
* @template {(node: import('#client').TemplateNode, ...args: any[]) => import('#client').Dom} SnippetFn * @template {(node: import('#client').TemplateNode, ...args: any[]) => import('#client').Dom} SnippetFn
@ -38,17 +42,17 @@ export function snippet(get_snippet, node, ...args) {
* @returns * @returns
*/ */
export function wrap_snippet(fn) { export function wrap_snippet(fn) {
let component = current_component_context; let component = /** @type {import('#client').ComponentContext} */ (current_component_context);
return add_snippet_symbol( return add_snippet_symbol(
(/** @type {import('#client').TemplateNode} */ node, /** @type {any[]} */ ...args) => { (/** @type {import('#client').TemplateNode} */ node, /** @type {any[]} */ ...args) => {
var previous_component_context = current_component_context; var previous_component_function = dev_current_component_function;
set_current_component_context(component); set_dev_current_component_function(component.function);
try { try {
return fn(node, ...args); return fn(node, ...args);
} finally { } finally {
set_current_component_context(previous_component_context); set_dev_current_component_function(previous_component_function);
} }
} }
); );

@ -1,5 +1,11 @@
export { hmr } from './dev/hmr.js'; export { hmr } from './dev/hmr.js';
export { ADD_OWNER, add_owner, mark_module_start, mark_module_end } from './dev/ownership.js'; export {
ADD_OWNER,
add_owner,
mark_module_start,
mark_module_end,
add_owner_effect
} from './dev/ownership.js';
export { inspect } from './dev/inspect.js'; export { inspect } from './dev/inspect.js';
export { await_block as await } from './dom/blocks/await.js'; export { await_block as await } from './dom/blocks/await.js';
export { if_block as if } from './dom/blocks/if.js'; export { if_block as if } from './dom/blocks/if.js';

@ -75,8 +75,7 @@ export function proxy(value, immutable = true, parent = null) {
value[STATE_SYMBOL].owners = value[STATE_SYMBOL].owners =
parent === null parent === null
? current_component_context !== null ? current_component_context !== null
? // @ts-expect-error ? new Set([current_component_context.function])
new Set([current_component_context.function])
: null : null
: new Set(); : new Set();
} }

@ -113,6 +113,26 @@ export let current_component_context = null;
/** @param {import('#client').ComponentContext | null} context */ /** @param {import('#client').ComponentContext | null} context */
export function set_current_component_context(context) { export function set_current_component_context(context) {
current_component_context = context; current_component_context = context;
if (DEV) {
dev_current_component_function = context?.function;
}
}
/**
* The current component function. Different from current component context:
* ```html
* <!-- App.svelte -->
* <Foo>
* <Bar /> <!-- context == Foo.svelte, function == App.svelte -->
* </Foo>
* ```
* @type {import('#client').ComponentContext['function']}
*/
export let dev_current_component_function = null;
/** @param {import('#client').ComponentContext['function']} fn */
export function set_dev_current_component_function(fn) {
dev_current_component_function = fn;
} }
/** @returns {boolean} */ /** @returns {boolean} */
@ -400,7 +420,7 @@ export function execute_effect(effect) {
var previous_component_context = current_component_context; var previous_component_context = current_component_context;
current_effect = effect; current_effect = effect;
current_component_context = component_context; set_current_component_context(component_context);
try { try {
if ((flags & BLOCK_EFFECT) === 0) { if ((flags & BLOCK_EFFECT) === 0) {
@ -412,7 +432,7 @@ export function execute_effect(effect) {
effect.teardown = typeof teardown === 'function' ? teardown : null; effect.teardown = typeof teardown === 'function' ? teardown : null;
} finally { } finally {
current_effect = previous_effect; current_effect = previous_effect;
current_component_context = previous_component_context; set_current_component_context(previous_component_context);
} }
} }
@ -885,8 +905,8 @@ export function getContext(key) {
const result = /** @type {T} */ (context_map.get(key)); const result = /** @type {T} */ (context_map.get(key));
if (DEV) { if (DEV) {
// @ts-expect-error const fn = /** @type {import('#client').ComponentContext} */ (current_component_context)
const fn = current_component_context.function; .function;
if (fn) { if (fn) {
add_owner(result, fn, true); add_owner(result, fn, true);
} }
@ -940,7 +960,6 @@ export function getAllContexts() {
const context_map = get_or_init_context_map('getAllContexts'); const context_map = get_or_init_context_map('getAllContexts');
if (DEV) { if (DEV) {
// @ts-expect-error
const fn = current_component_context?.function; const fn = current_component_context?.function;
if (fn) { if (fn) {
for (const value of context_map.values()) { for (const value of context_map.values()) {
@ -1066,8 +1085,8 @@ export function push(props, runes = false, fn) {
if (DEV) { if (DEV) {
// component function // component function
// @ts-expect-error
current_component_context.function = fn; current_component_context.function = fn;
dev_current_component_function = fn;
} }
} }
@ -1089,7 +1108,7 @@ export function pop(component) {
effect(effects[i]); effect(effects[i]);
} }
} }
current_component_context = context_stack_item.p; set_current_component_context(context_stack_item.p);
context_stack_item.m = true; context_stack_item.m = true;
} }
// Micro-optimization: Don't set .a above to the empty object // Micro-optimization: Don't set .a above to the empty object

@ -49,6 +49,10 @@ export type ComponentContext = {
/** This tracks whether `$:` statements have run in the current cycle, to ensure they only run once */ /** This tracks whether `$:` statements have run in the current cycle, to ensure they only run once */
r2: Source<boolean>; r2: Source<boolean>;
}; };
/**
* dev mode only: the component function
*/
function?: any;
}; };
export type ComponentContextLegacy = ComponentContext & { export type ComponentContextLegacy = ComponentContext & {

@ -1,6 +1,9 @@
import { test } from '../../test'; import { test } from '../../test';
export default test({ export default test({
compileOptions: {
dev: true // to ensure dev mode does not break context in some way
},
html: ` html: `
<div class="tabs"> <div class="tabs">
<div class="tab-list"> <div class="tab-list">

Loading…
Cancel
Save