diff --git a/.changeset/slimy-doors-fetch.md b/.changeset/slimy-doors-fetch.md new file mode 100644 index 000000000000..8dec24a98dab --- /dev/null +++ b/.changeset/slimy-doors-fetch.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: more informative error when effects run in an infinite loop diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index c7d6ec8ac9cd..3b17ef9f9b4e 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/documentation/docs/98-reference/.generated/client-errors.md @@ -89,9 +89,47 @@ Effect cannot be created inside a `$derived` value that was not itself created i ### effect_update_depth_exceeded ``` -Maximum update depth exceeded. This can happen when a reactive block or effect repeatedly sets a new value. Svelte limits the number of nested updates to prevent infinite loops +Maximum update depth exceeded. This typically indicates that an effect reads and writes the same piece of state ``` +If an effect updates some state that it also depends on, it will re-run, potentially in a loop: + +```js +let count = $state(0); + +$effect(() => { + // this both reads and writes `count`, + // so will run in an infinite loop + count += 1; +}); +``` + +(Svelte intervenes before this can crash your browser tab.) + +The same applies to array mutations, since these both read and write to the array: + +```js +let array = $state([]); + +$effect(() => { + array.push('hello'); +}); +``` + +Note that it's fine for an effect to re-run itself as long as it 'settles': + +```js +let array = ['a', 'b', 'c']; +// ---cut--- +$effect(() => { + // this is okay, because sorting an already-sorted array + // won't result in a mutation + array.sort(); +}); +``` + +Often when encountering this issue, the value in question shouldn't be state (for example, if you are pushing to a `logs` array in an effect, make `logs` a normal array rather than `$state([])`). In the rare cases where you really _do_ need to write to state in an effect — [which you should avoid]($effect#When-not-to-use-$effect) — you can read the state with [untrack](svelte#untrack) to avoid adding it as a dependency. + ### flush_sync_in_effect ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 2ce25dbd53e6..d6af8598812b 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -60,7 +60,45 @@ See the [migration guide](/docs/svelte/v5-migration-guide#Components-are-no-long ## effect_update_depth_exceeded -> Maximum update depth exceeded. This can happen when a reactive block or effect repeatedly sets a new value. Svelte limits the number of nested updates to prevent infinite loops +> Maximum update depth exceeded. This typically indicates that an effect reads and writes the same piece of state + +If an effect updates some state that it also depends on, it will re-run, potentially in a loop: + +```js +let count = $state(0); + +$effect(() => { + // this both reads and writes `count`, + // so will run in an infinite loop + count += 1; +}); +``` + +(Svelte intervenes before this can crash your browser tab.) + +The same applies to array mutations, since these both read and write to the array: + +```js +let array = $state([]); + +$effect(() => { + array.push('hello'); +}); +``` + +Note that it's fine for an effect to re-run itself as long as it 'settles': + +```js +let array = ['a', 'b', 'c']; +// ---cut--- +$effect(() => { + // this is okay, because sorting an already-sorted array + // won't result in a mutation + array.sort(); +}); +``` + +Often when encountering this issue, the value in question shouldn't be state (for example, if you are pushing to a `logs` array in an effect, make `logs` a normal array rather than `$state([])`). In the rare cases where you really _do_ need to write to state in an effect — [which you should avoid]($effect#When-not-to-use-$effect) — you can read the state with [untrack](svelte#untrack) to avoid adding it as a dependency. ## flush_sync_in_effect diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index 128942ceb293..b7a6a385486e 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -56,8 +56,10 @@ function log_entry(signal, entry) { } if (dirty && signal.updated) { - // eslint-disable-next-line no-console - console.log(signal.updated); + for (const updated of signal.updated.values()) { + // eslint-disable-next-line no-console + console.log(updated.error); + } } if (entry) { @@ -120,44 +122,46 @@ export function trace(label, fn) { /** * @param {string} label + * @returns {Error & { stack: string } | null} */ export function get_stack(label) { let error = Error(); const stack = error.stack; - if (stack) { - const lines = stack.split('\n'); - const new_lines = ['\n']; - - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - - if (line === 'Error') { - continue; - } - if (line.includes('validate_each_keys')) { - return null; - } - if (line.includes('svelte/src/internal')) { - continue; - } - new_lines.push(line); - } + if (!stack) return null; - if (new_lines.length === 1) { + const lines = stack.split('\n'); + const new_lines = ['\n']; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + if (line === 'Error') { + continue; + } + if (line.includes('validate_each_keys')) { return null; } + if (line.includes('svelte/src/internal')) { + continue; + } + new_lines.push(line); + } - define_property(error, 'stack', { - value: new_lines.join('\n') - }); - - define_property(error, 'name', { - // 'Error' suffix is required for stack traces to be rendered properly - value: `${label}Error` - }); + if (new_lines.length === 1) { + return null; } - return error; + + define_property(error, 'stack', { + value: new_lines.join('\n') + }); + + define_property(error, 'name', { + // 'Error' suffix is required for stack traces to be rendered properly + value: `${label}Error` + }); + + return /** @type {Error & { stack: string }} */ (error); } /** diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index a491dc683d9e..edd66a7400d7 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -214,12 +214,12 @@ export function effect_pending_outside_reaction() { } /** - * Maximum update depth exceeded. This can happen when a reactive block or effect repeatedly sets a new value. Svelte limits the number of nested updates to prevent infinite loops + * Maximum update depth exceeded. This typically indicates that an effect reads and writes the same piece of state * @returns {never} */ export function effect_update_depth_exceeded() { if (DEV) { - const error = new Error(`effect_update_depth_exceeded\nMaximum update depth exceeded. This can happen when a reactive block or effect repeatedly sets a new value. Svelte limits the number of nested updates to prevent infinite loops\nhttps://svelte.dev/e/effect_update_depth_exceeded`); + const error = new Error(`effect_update_depth_exceeded\nMaximum update depth exceeded. This typically indicates that an effect reads and writes the same piece of state\nhttps://svelte.dev/e/effect_update_depth_exceeded`); error.name = 'Svelte error'; diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 1d08b5c3d82b..cdce971b189a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -46,9 +46,6 @@ export let current_batch = null; */ export let batch_deriveds = null; -/** @type {Effect[]} Stack of effects, dev only */ -export let dev_effect_stack = []; - /** @type {Set<() => void>} */ export let effect_pending_updates = new Set(); @@ -345,6 +342,28 @@ export class Batch { while (queued_root_effects.length > 0) { if (flush_count++ > 1000) { + if (DEV) { + var updates = new Map(); + + for (const source of this.#current.keys()) { + for (const [stack, update] of source.updated ?? []) { + var entry = updates.get(stack); + + if (!entry) { + entry = { error: update.error, count: 0 }; + updates.set(stack, entry); + } + + entry.count += update.count; + } + } + + for (const update of updates.values()) { + // eslint-disable-next-line no-console + console.error(update.error); + } + } + infinite_loop_guard(); } @@ -356,9 +375,6 @@ export class Batch { set_is_updating_effect(was_updating_effect); last_scheduled_effect = null; - if (DEV) { - dev_effect_stack = []; - } } } @@ -471,10 +487,6 @@ export function flushSync(fn) { // we need to reset it here as well in case the first time there's 0 queued root effects last_scheduled_effect = null; - if (DEV) { - dev_effect_stack = []; - } - return /** @type {T} */ (result); } @@ -482,45 +494,18 @@ export function flushSync(fn) { } } -function log_effect_stack() { - // eslint-disable-next-line no-console - console.error( - 'Last ten effects were: ', - dev_effect_stack.slice(-10).map((d) => d.fn) - ); - dev_effect_stack = []; -} - function infinite_loop_guard() { try { e.effect_update_depth_exceeded(); } catch (error) { if (DEV) { - // stack is garbage, ignore. Instead add a console.error message. - define_property(error, 'stack', { - value: '' - }); - } - // Try and handle the error so it can be caught at a boundary, that's - // if there's an effect available from when it was last scheduled - if (last_scheduled_effect !== null) { - if (DEV) { - try { - invoke_error_boundary(error, last_scheduled_effect); - } catch (e) { - // Only log the effect stack if the error is re-thrown - log_effect_stack(); - throw e; - } - } else { - invoke_error_boundary(error, last_scheduled_effect); - } - } else { - if (DEV) { - log_effect_stack(); - } - throw error; + // stack contains no useful information, replace it + define_property(error, 'stack', { value: '' }); } + + // Best effort: invoke the boundary nearest the most recent + // effect and hope that it's relevant to the infinite loop + invoke_error_boundary(error, last_scheduled_effect); } } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index bd55b9d935f4..9b534d2d7190 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -182,8 +182,22 @@ export function internal_set(source, value) { const batch = Batch.ensure(); batch.capture(source, old_value); - if (DEV && tracing_mode_flag) { - source.updated = get_stack('UpdatedAt'); + if (DEV) { + if (tracing_mode_flag || active_effect !== null) { + const error = get_stack('UpdatedAt'); + + if (error !== null) { + source.updated ??= new Map(); + let entry = source.updated.get(error.stack); + + if (!entry) { + entry = { error, count: 0 }; + source.updated.set(error.stack, entry); + } + + entry.count++; + } + } if (active_effect !== null) { source.set_during_effect = true; diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 90f922df686b..72187e84a720 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -29,8 +29,8 @@ export interface Value extends Signal { label?: string; /** An error with a stack trace showing when the source was created */ created?: Error | null; - /** An error with a stack trace showing when the source was last updated */ - updated?: Error | null; + /** An map of errors with stack traces showing when the source was updated, keyed by the stack trace */ + updated?: Map | null; /** * Whether or not the source was set while running an effect — if so, we need to * increment the write version so that it shows up as dirty when the effect re-runs diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 6c4d92bbadf5..306b9b9dd9a1 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -42,13 +42,7 @@ import { set_dev_stack } from './context.js'; import * as w from './warnings.js'; -import { - Batch, - batch_deriveds, - dev_effect_stack, - flushSync, - schedule_effect -} from './reactivity/batch.js'; +import { Batch, batch_deriveds, flushSync, schedule_effect } from './reactivity/batch.js'; import { handle_error } from './error-handling.js'; import { UNINITIALIZED } from '../../constants.js'; @@ -491,10 +485,6 @@ export function update_effect(effect) { } } } - - if (DEV) { - dev_effect_stack.push(effect); - } } finally { is_updating_effect = was_updating_effect; active_effect = previous_effect; diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/_config.js new file mode 100644 index 000000000000..400495050cfd --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/_config.js @@ -0,0 +1,21 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['client', 'hydrate'], + + compileOptions: { + dev: true + }, + + test({ assert, errors }) { + const [button] = document.querySelectorAll('button'); + + try { + flushSync(() => button.click()); + } catch (e) { + assert.equal(errors.length, 1); // for whatever reason we can't get the name which should be UpdatedAtError + assert.ok(/** @type {Error} */ (e).message.startsWith('effect_update_depth_exceeded')); + } + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/main.svelte new file mode 100644 index 000000000000..ddb91a90ad22 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-infinite/main.svelte @@ -0,0 +1,12 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js index ccff614ade0d..e3a3b0c7d752 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js @@ -2,12 +2,14 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; export default test({ - test({ assert, target }) { + test({ assert, target, errors }) { let btn = target.querySelector('button'); btn?.click(); flushSync(); + assert.equal(errors.length, 1); + assert.htmlEqual(target.innerHTML, `
An error occurred!
`); } });