From 16c7671f77b9dbe5ec28a4858e67cb662fa5f2dc Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 23 Dec 2024 19:57:21 +0100 Subject: [PATCH 1/5] fix: correctly highlight sources reassigned inside `trace` --- .changeset/silver-shoes-rule.md | 5 ++ .../svelte/src/internal/client/dev/tracing.js | 2 +- .../src/internal/client/reactivity/sources.js | 5 ++ .../src/internal/client/reactivity/types.d.ts | 2 + .../svelte/src/internal/client/runtime.js | 17 +++++ .../inspect-trace-reassignment/_config.js | 70 +++++++++++++++++++ .../inspect-trace-reassignment/main.svelte | 18 +++++ 7 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 .changeset/silver-shoes-rule.md create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/main.svelte diff --git a/.changeset/silver-shoes-rule.md b/.changeset/silver-shoes-rule.md new file mode 100644 index 000000000000..b1c44c900703 --- /dev/null +++ b/.changeset/silver-shoes-rule.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly highlight sources reassigned inside `trace` diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index 1426e9efc9ed..20e599a81f68 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -15,7 +15,7 @@ export let tracing_expressions = null; */ function log_entry(signal, entry) { const debug = signal.debug; - const value = signal.v; + const value = signal.trace_need_increase ? signal.trace_v : signal.v; if (value === UNINITIALIZED) { return; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 3e8c4a00c833..fedf5b50701e 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -167,11 +167,16 @@ export function set(source, value) { */ export function internal_set(source, value) { if (!source.equals(value)) { + var old_value = source.v; source.v = value; source.version = increment_version(); if (DEV && tracing_mode_flag) { source.updated = get_stack('UpdatedAt'); + if (active_effect != null) { + source.trace_need_increase = true; + source.trace_v ??= old_value; + } } mark_reactions(source, DIRTY); diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 13e14414b519..9a6e5d8bf098 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -17,6 +17,8 @@ export interface Value extends Signal { /** Dev only */ created?: Error | null; updated?: Error | null; + trace_need_increase?: boolean; + trace_v?: V; debug?: null | (() => void); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 4a90a219712f..e46ce87de273 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -528,6 +528,23 @@ export function update_effect(effect) { effect.teardown = typeof teardown === 'function' ? teardown : null; effect.version = current_version; + var deps = effect.deps; + + // In DEV, we need to handle a case where $inspect.trace() might + // incorrectly state a source dependency has not changed when it has. + // That's beacuse that source was changed by the same effect, causing + // the versions to match. We can avoid this by incrementing the version + if (DEV && tracing_mode_flag && (effect.f & DIRTY) !== 0 && deps !== null) { + for (let i = 0; i < deps.length; i++) { + var dep = deps[i]; + if (dep.trace_need_increase) { + dep.version = increment_version(); + dep.trace_need_increase = undefined; + dep.trace_v = undefined; + } + } + } + if (DEV) { dev_effect_stack.push(effect); } diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js new file mode 100644 index 000000000000..152d564fe597 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js @@ -0,0 +1,70 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +/** + * @param {any[]} logs + */ +function normalise_trace_logs(logs) { + let normalised = []; + for (let i = 0; i < logs.length; i++) { + const log = logs[i]; + + if (typeof log === 'string' && log.includes('%c')) { + const split = log.split('%c'); + normalised.push({ + log: (split[0].length !== 0 ? split[0] : split[1]).trim(), + highlighted: logs[i + 1] === 'color: CornflowerBlue; font-weight: bold' + }); + i++; + } else if (log instanceof Error) { + continue; + } else { + normalised.push({ log }); + } + } + return normalised; +} + +export default test({ + compileOptions: { + dev: true + }, + + test({ assert, target, logs }) { + // initial log, everything is highlighted + + assert.deepEqual(normalise_trace_logs(logs), [ + { log: 'effect', highlighted: false }, + { log: '$state', highlighted: true }, + { log: false } + ]); + + logs.length = 0; + + const button = target.querySelector('button'); + button?.click(); + flushSync(); + + const input = target.querySelector('input'); + input?.click(); + flushSync(); + + // checked changed, effect reassign state, values should be correct and be correctly highlighted + + assert.deepEqual(normalise_trace_logs(logs), [ + { log: 'effect', highlighted: false }, + { log: '$state', highlighted: true }, + { log: true }, + { log: '$state', highlighted: true }, + { log: 1 }, + { log: '$state', highlighted: false }, + { log: true }, + { log: '$state', highlighted: true }, + { log: 2 }, + { log: '$state', highlighted: false }, + { log: true }, + { log: '$state', highlighted: true }, + { log: 3 } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/main.svelte new file mode 100644 index 000000000000..5028c0f25183 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/main.svelte @@ -0,0 +1,18 @@ + + + \ No newline at end of file From cfd10c25c2aec6aaab8bf3206e9797178845879b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 23 Dec 2024 21:13:54 +0100 Subject: [PATCH 2/5] chore: add missing effect logs --- .../runtime-runes/samples/inspect-trace-reassignment/_config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js index 152d564fe597..c9a66289a12d 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js @@ -57,10 +57,12 @@ export default test({ { log: true }, { log: '$state', highlighted: true }, { log: 1 }, + { log: 'effect', highlighted: false }, { log: '$state', highlighted: false }, { log: true }, { log: '$state', highlighted: true }, { log: 2 }, + { log: 'effect', highlighted: false }, { log: '$state', highlighted: false }, { log: true }, { log: '$state', highlighted: true }, From 310d4baf0ba664932a5e003b2e2e48ea20f266d5 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 24 Dec 2024 13:05:02 +0100 Subject: [PATCH 3/5] fix: prevent `null` access on `tracing_expressions` for nested tracing --- .../svelte/src/internal/client/dev/tracing.js | 2 +- .../samples/inspect-trace-nested/_config.js | 56 +++++++++++++++++++ .../samples/inspect-trace-nested/main.svelte | 13 +++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/main.svelte diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index 20e599a81f68..79edb59b3978 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -119,7 +119,7 @@ export function trace(label, fn) { console.groupEnd(); } - if (previously_tracing_expressions !== null) { + if (previously_tracing_expressions !== null && tracing_expressions !== null) { for (const [signal, entry] of tracing_expressions.entries) { var prev_entry = previously_tracing_expressions.get(signal); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/_config.js new file mode 100644 index 000000000000..f54f78f5c1f3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/_config.js @@ -0,0 +1,56 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +/** + * @param {any[]} logs + */ +function normalise_trace_logs(logs) { + let normalised = []; + for (let i = 0; i < logs.length; i++) { + const log = logs[i]; + + if (typeof log === 'string' && log.includes('%c')) { + const split = log.split('%c'); + normalised.push({ + log: (split[0].length !== 0 ? split[0] : split[1]).trim(), + highlighted: logs[i + 1] === 'color: CornflowerBlue; font-weight: bold' + }); + i++; + } else if (log instanceof Error) { + continue; + } else { + normalised.push({ log }); + } + } + return normalised; +} + +export default test({ + compileOptions: { + dev: true + }, + + test({ assert, target, logs }) { + // initial log, everything is highlighted + + assert.deepEqual(normalise_trace_logs(logs), [ + { log: 'iife', highlighted: false }, + { log: '$state', highlighted: true }, + { log: 0 }, + { log: 'effect', highlighted: false } + ]); + + logs.length = 0; + + const button = target.querySelector('button'); + button?.click(); + flushSync(); + + assert.deepEqual(normalise_trace_logs(logs), [ + { log: 'iife', highlighted: false }, + { log: '$state', highlighted: true }, + { log: 1 }, + { log: 'effect', highlighted: false } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/main.svelte new file mode 100644 index 000000000000..f60fb0438c3b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/main.svelte @@ -0,0 +1,13 @@ + + + From 35c8f6d491511ff8c4dd3afe84b3b34897624c5b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 29 Dec 2024 10:39:53 +0100 Subject: [PATCH 4/5] chore: add test case for #14853 --- .../runtime-runes/samples/inspect-trace-null/_config.js | 8 ++++++++ .../runtime-runes/samples/inspect-trace-null/main.svelte | 9 +++++++++ 2 files changed, 17 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-null/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-trace-null/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/_config.js new file mode 100644 index 000000000000..e779a835c2ef --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + test() {} +}); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/main.svelte new file mode 100644 index 000000000000..30eb95f12418 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-trace-null/main.svelte @@ -0,0 +1,9 @@ + + + \ No newline at end of file From 83473d3f9580c9a88914cb5431ebba0f1218245b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 2 Jan 2025 17:36:02 +0100 Subject: [PATCH 5/5] fix: types for `$inpect.trace` --- packages/svelte/src/ambient.d.ts | 2 +- packages/svelte/types/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/ambient.d.ts b/packages/svelte/src/ambient.d.ts index 9dbc61c7cbd4..fbcecba8e47c 100644 --- a/packages/svelte/src/ambient.d.ts +++ b/packages/svelte/src/ambient.d.ts @@ -433,7 +433,7 @@ declare namespace $inspect { * }); * */ - export function trace(name: string): void; + export function trace(name?: string): void; // prevent intellisense from being unhelpful /** @deprecated */ diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index d422abebbc0f..b6b76b1cb7e2 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -3091,7 +3091,7 @@ declare namespace $inspect { * }); * */ - export function trace(name: string): void; + export function trace(name?: string): void; // prevent intellisense from being unhelpful /** @deprecated */