Skip to content

Commit

Permalink
fix: widen ownership when calling setContext (#15153)
Browse files Browse the repository at this point in the history
Instead of doing ownership addition at each `getContext` call site, we do it once as the `setContext` call site and make the state basically global.
- avoids potential false positives in edge cases
- makes the whole thing more performant, especially around things like calling `getContext` inside a list item component
- false negatives are unlikely from this

fixes #15072
  • Loading branch information
dummdidumm authored Jan 30, 2025
1 parent 5225575 commit f5406c9
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/unlucky-gorillas-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: widen ownership when calling setContext
30 changes: 11 additions & 19 deletions packages/svelte/src/internal/client/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
active_effect,
active_reaction,
set_active_effect,
set_active_reaction
set_active_reaction,
untrack
} from './runtime.js';
import { effect } from './reactivity/effects.js';
import { legacy_mode_flag } from '../flags/index.js';
Expand Down Expand Up @@ -49,14 +50,6 @@ export function set_dev_current_component_function(fn) {
export function getContext(key) {
const context_map = get_or_init_context_map('getContext');
const result = /** @type {T} */ (context_map.get(key));

if (DEV) {
const fn = /** @type {ComponentContext} */ (component_context).function;
if (fn) {
add_owner(result, fn, true);
}
}

return result;
}

Expand All @@ -74,6 +67,15 @@ 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;
}
Expand All @@ -100,16 +102,6 @@ export function hasContext(key) {
*/
export function getAllContexts() {
const context_map = get_or_init_context_map('getAllContexts');

if (DEV) {
const fn = component_context?.function;
if (fn) {
for (const value of context_map.values()) {
add_owner(value, fn, true);
}
}
}

return /** @type {T} */ (context_map);
}

Expand Down
12 changes: 8 additions & 4 deletions packages/svelte/src/internal/client/dev/ownership.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export function mark_module_end(component) {

/**
* @param {any} object
* @param {any} owner
* @param {any | null} owner
* @param {boolean} [global]
* @param {boolean} [skip_warning]
*/
Expand All @@ -120,7 +120,7 @@ export function add_owner(object, owner, global = false, skip_warning = false) {
if (metadata && !has_owner(metadata, component)) {
let original = get_owner(metadata);

if (owner[FILENAME] !== component[FILENAME] && !skip_warning) {
if (owner && owner[FILENAME] !== component[FILENAME] && !skip_warning) {
w.ownership_invalid_binding(component[FILENAME], owner[FILENAME], original[FILENAME]);
}
}
Expand Down Expand Up @@ -165,7 +165,7 @@ export function widen_ownership(from, to) {

/**
* @param {any} object
* @param {Function} owner
* @param {Function | null} owner If `null`, then the object is globally owned and will not be checked
* @param {Set<any>} seen
*/
function add_owner_to_object(object, owner, seen) {
Expand All @@ -174,7 +174,11 @@ function add_owner_to_object(object, owner, seen) {
if (metadata) {
// this is a state proxy, add owner directly, if not globally shared
if ('owners' in metadata && metadata.owners != null) {
metadata.owners.add(owner);
if (owner) {
metadata.owners.add(owner);
} else {
metadata.owners = null;
}
}
} else if (object && typeof object === 'object') {
if (seen.has(object)) return;
Expand Down

0 comments on commit f5406c9

Please sign in to comment.