Skip to content

Commit

Permalink
fix: deeply unstate objects passed to inspect (#10056)
Browse files Browse the repository at this point in the history
When doing `$inspect({ x, y })`, both `x` and `y` are now unstated if they are signals, compared to before where `unstate` was only called on the top level object, leaving the proxies in place which results in a worse debugging experience.
Also improved typings which makes it easier to find related code paths.
  • Loading branch information
dummdidumm committed Jan 2, 2024
1 parent e46a71e commit 8a85059
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 11 deletions.
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>

1 comment on commit 8a85059

@vercel
Copy link

@vercel vercel bot commented on 8a85059 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

svelte-5-preview – ./sites/svelte-5-preview

svelte-5-preview.vercel.app
svelte-5-preview-svelte.vercel.app
svelte-octane.vercel.app
svelte-5-preview-git-main-svelte.vercel.app

Please sign in to comment.