Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: deeply unstate objects passed to inspect #10056

Merged
merged 1 commit into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curvy-ties-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: deeply unstate objects passed to inspect
49 changes: 38 additions & 11 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ let current_dependencies = null;
let current_dependencies_index = 0;
/** @type {null | import('./types.js').Signal[]} */
let current_untracked_writes = null;
/** @type {null | import('./types.js').Signal} */
/** @type {null | import('./types.js').SignalDebug} */
let last_inspected_signal = null;
/** If `true`, `get`ting the signal should not register it as a dependency */
export let current_untracking = false;
Expand All @@ -81,7 +81,7 @@ let captured_signals = new Set();
/** @type {Function | null} */
let inspect_fn = null;

/** @type {Array<import('./types.js').SourceSignal & import('./types.js').SourceSignalDebug>} */
/** @type {Array<import('./types.js').SignalDebug>} */
let inspect_captured_signals = [];

// Handle rendering tree blocks and anchors
Expand Down Expand Up @@ -127,7 +127,6 @@ export function batch_inspect(target, prop, receiver) {
} finally {
is_batching_effect = previously_batching_effect;
if (last_inspected_signal !== null) {
// @ts-expect-error
for (const fn of last_inspected_signal.inspect) fn();
last_inspected_signal = null;
}
Expand Down Expand Up @@ -737,8 +736,7 @@ function update_derived(signal, force_schedule) {

// @ts-expect-error
if (DEV && signal.inspect && force_schedule) {
// @ts-expect-error
for (const fn of signal.inspect) fn();
for (const fn of /** @type {import('./types.js').SignalDebug} */ (signal).inspect) fn();
}
}
}
Expand Down Expand Up @@ -841,8 +839,7 @@ export function unsubscribe_on_destroy(stores) {
export function get(signal) {
// @ts-expect-error
if (DEV && signal.inspect && inspect_fn) {
// @ts-expect-error
signal.inspect.add(inspect_fn);
/** @type {import('./types.js').SignalDebug} */ (signal).inspect.add(inspect_fn);
// @ts-expect-error
inspect_captured_signals.push(signal);
}
Expand Down Expand Up @@ -1111,10 +1108,9 @@ export function set_signal_value(signal, value) {
// @ts-expect-error
if (DEV && signal.inspect) {
if (is_batching_effect) {
last_inspected_signal = signal;
last_inspected_signal = /** @type {import('./types.js').SignalDebug} */ (signal);
} else {
// @ts-expect-error
for (const fn of signal.inspect) fn();
for (const fn of /** @type {import('./types.js').SignalDebug} */ (signal).inspect) fn();
}
}
}
Expand Down Expand Up @@ -1836,6 +1832,37 @@ function deep_read(value, visited = new Set()) {
}
}

/**
* Like `unstate`, but recursively traverses into normal arrays/objects to find potential states in them.
* @param {any} value
* @param {Map<any, any>} visited
* @returns {any}
*/
function deep_unstate(value, visited = new Map()) {
if (typeof value === 'object' && value !== null && !visited.has(value)) {
const unstated = unstate(value);
if (unstated !== value) {
visited.set(value, unstated);
return unstated;
}

let contains_unstated = false;
/** @type {any} */
const nested_unstated = Array.isArray(value) ? [] : {};
for (let key in value) {
const result = deep_unstate(value[key], visited);
nested_unstated[key] = result;
if (result !== value[key]) {
contains_unstated = true;
}
}

visited.set(value, contains_unstated ? nested_unstated : value);
}

return visited.get(value) ?? value;
}

// TODO remove in a few versions, before 5.0 at the latest
let warned_inspect_changed = false;

Expand All @@ -1849,7 +1876,7 @@ export function inspect(get_value, inspect = console.log) {

pre_effect(() => {
const fn = () => {
const value = get_value().map(unstate);
const value = get_value().map((v) => deep_unstate(v));
if (value.length === 2 && typeof value[1] === 'function' && !warned_inspect_changed) {
// eslint-disable-next-line no-console
console.warn(
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/src/internal/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ export type ComputationSignal<V = unknown> = {

export type Signal<V = unknown> = SourceSignal<V> | ComputationSignal<V>;

export type SignalDebug<V = unknown> = SourceSignalDebug & Signal<V>;

export type EffectSignal = ComputationSignal<null | (() => void)>;

export type MaybeSignal<T = unknown> = T | Signal<T>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { test } from '../../test';

/**
* @type {any[]}
*/
let log;
/**
* @type {typeof console.log}}
*/
let original_log;

export default test({
compileOptions: {
dev: true
},
before_test() {
log = [];
original_log = console.log;
console.log = (...v) => {
log.push(...v);
};
},
after_test() {
console.log = original_log;
},
async test({ assert, target }) {
const [b1] = target.querySelectorAll('button');
b1.click();
await Promise.resolve();

assert.deepEqual(log, [
'init',
{ x: { count: 0 } },
[{ count: 0 }],
'update',
{ x: { count: 1 } },
[{ count: 1 }]
]);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let x = $state({count: 0});

$inspect({x}, [x]);
</script>

<button on:click={() => x.count++}>{x.count}</button>