From cc2469cb0db7db5877c80a8f887fff7a59d63cc1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 19 Sep 2024 23:44:43 -0400 Subject: [PATCH] track assignment locations --- .../svelte/messages/client-errors/errors.md | 4 ++- .../client/visitors/AssignmentExpression.js | 18 ++++++++-- .../client/visitors/shared/declarations.js | 28 +++++++++++++-- .../internal/client/dev/assignment-stack.js | 9 +++++ .../svelte/src/internal/client/dev/utils.js | 7 ++++ .../src/internal/client/dom/blocks/html.js | 5 ++- packages/svelte/src/internal/client/errors.js | 7 ++-- packages/svelte/src/internal/client/index.js | 1 + .../svelte/src/internal/client/runtime.js | 34 ++++++------------- 9 files changed, 78 insertions(+), 35 deletions(-) create mode 100644 packages/svelte/src/internal/client/dev/assignment-stack.js create mode 100644 packages/svelte/src/internal/client/dev/utils.js diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 9bd7e8a654a6..3e595e6dca4a 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -42,7 +42,9 @@ ## 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 usually indicates state is being updated inside an effect, which you should avoid. Svelte limits the number of nested updates to prevent infinite loops + +> Maximum update depth exceeded after assignment at %location%. This usually indicates state is being updated inside an effect that also depends on that state. Svelte limits the number of nested updates to prevent infinite loops ## hydration_failed diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index cf54dba1b32a..9abfb23a8f50 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -1,8 +1,9 @@ /** @import { AssignmentExpression, AssignmentOperator, Expression, Pattern } from 'estree' */ +/** @import { Location } from 'locate-character' */ /** @import { Context } from '../types.js' */ import * as b from '../../../../utils/builders.js'; import { build_assignment_value } from '../../../../utils/ast.js'; -import { is_ignored } from '../../../../state.js'; +import { dev, filename, is_ignored, locator } from '../../../../state.js'; import { build_proxy_reassignment, should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; @@ -11,10 +12,23 @@ import { visit_assignment_expression } from '../../shared/assignments.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - const expression = /** @type {Expression} */ ( + let expression = /** @type {Expression} */ ( visit_assignment_expression(node, context, build_assignment) ?? context.next() ); + const loc = + dev && + node.left.type === 'MemberExpression' && + node.left.start !== undefined && + locator(node.left.start); + + if (loc) { + expression = b.sequence([ + b.call('$.track_assignment', b.literal(`${filename}:${loc.line}:${loc.column}`)), + expression + ]); + } + return is_ignored(node, 'ownership_invalid_mutation') ? b.call('$.skip_ownership_validation', b.thunk(expression)) : expression; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js index 0bd8c352f6a9..d6f98193b011 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -1,7 +1,9 @@ -/** @import { Identifier } from 'estree' */ +/** @import { Expression, Identifier } from 'estree' */ +/** @import { Location } from 'locate-character' */ /** @import { ComponentContext, Context } from '../../types' */ import { is_state_source } from '../../utils.js'; import * as b from '../../../../../utils/builders.js'; +import { dev, filename, locator } from '../../../../../state.js'; /** * Turns `foo` into `$.get(foo)` @@ -25,8 +27,18 @@ export function add_state_transformers(context) { context.state.transform[name] = { read: binding.declaration_kind === 'var' ? (node) => b.call('$.safe_get', node) : get_value, assign: (node, value) => { + /** @type {Expression} */ let call = b.call('$.set', node, value); + const loc = dev && node.start !== undefined && locator(node.start); + + if (loc) { + call = b.sequence([ + b.call('$.track_assignment', b.literal(`${filename}:${loc.line}:${loc.column}`)), + call + ]); + } + if (context.state.scope.get(`$${node.name}`)?.kind === 'store_sub') { call = b.call('$.store_unsub', call, b.literal(`$${node.name}`), b.id('$$stores')); } @@ -41,11 +53,23 @@ export function add_state_transformers(context) { return b.call('$.mutate', node, mutation); }, update: (node) => { - return b.call( + /** @type {Expression} */ + let call = b.call( node.prefix ? '$.update_pre' : '$.update', node.argument, node.operator === '--' && b.literal(-1) ); + + const loc = dev && node.start !== undefined && locator(node.start); + + if (loc) { + call = b.sequence([ + b.call('$.track_assignment', b.literal(`${filename}:${loc.line}:${loc.column}`)), + call + ]); + } + + return call; } }; } diff --git a/packages/svelte/src/internal/client/dev/assignment-stack.js b/packages/svelte/src/internal/client/dev/assignment-stack.js new file mode 100644 index 000000000000..19bdd35f2712 --- /dev/null +++ b/packages/svelte/src/internal/client/dev/assignment-stack.js @@ -0,0 +1,9 @@ +/** @type {string[]} */ +export const assignment_stack = []; + +/** + * @param {string} location + */ +export function track_assignment(location) { + assignment_stack.push(location); +} diff --git a/packages/svelte/src/internal/client/dev/utils.js b/packages/svelte/src/internal/client/dev/utils.js new file mode 100644 index 000000000000..fc91866e2fc6 --- /dev/null +++ b/packages/svelte/src/internal/client/dev/utils.js @@ -0,0 +1,7 @@ +/** + * Append zero-width space to '/' characters to prevent devtools trying to make locations clickable + * @param {string} location + */ +export function sanitize_location(location) { + return location?.replace(/\//g, '/\u200b'); +} diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index aa13336b296b..962f676e821b 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -9,6 +9,7 @@ import { hash } from '../../../../utils.js'; import { DEV } from 'esm-env'; import { dev_current_component_function } from '../../runtime.js'; import { get_first_child, get_next_sibling } from '../operations.js'; +import { sanitize_location } from '../../dev/utils.js'; /** * @param {Element} element @@ -28,9 +29,7 @@ function check_hash(element, server_hash, value) { location = `in ${dev_current_component_function[FILENAME]}`; } - w.hydration_html_changed( - location?.replace(/\//g, '/\u200b') // prevent devtools trying to make it a clickable link by inserting a zero-width space - ); + w.hydration_html_changed(location && sanitize_location(location)); } /** diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 26d38f0aba91..dd796526ec52 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -179,12 +179,13 @@ export function effect_orphan(rune) { } /** - * 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 after assignment at %location%. This usually indicates state is being updated inside an effect that also depends on that state. Svelte limits the number of nested updates to prevent infinite loops + * @param {string | undefined | null} [location] * @returns {never} */ -export function effect_update_depth_exceeded() { +export function effect_update_depth_exceeded(location) { 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`); + const error = new Error(`effect_update_depth_exceeded\n${location ? `Maximum update depth exceeded after assignment at ${location}. This usually indicates state is being updated inside an effect that also depends on that state. Svelte limits the number of nested updates to prevent infinite loops` : "Maximum update depth exceeded. This usually indicates state is being updated inside an effect, which you should avoid. Svelte limits the number of nested updates to prevent infinite loops"}`); error.name = 'Svelte error'; throw error; diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 0debfe9d191f..0978b805c4ca 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -1,4 +1,5 @@ export { FILENAME, HMR } from '../../constants.js'; +export { track_assignment } from './dev/assignment-stack.js'; export { cleanup_styles } from './dev/css.js'; export { add_locations } from './dev/elements.js'; export { hmr } from './dev/hmr.js'; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 93f8164ba3de..baf19d0c8143 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -31,6 +31,8 @@ import { update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { lifecycle_outside_component } from '../shared/errors.js'; import { FILENAME } from '../../constants.js'; +import { assignment_stack } from './dev/assignment-stack.js'; +import { sanitize_location } from './dev/utils.js'; const FLUSH_MICROTASK = 0; const FLUSH_SYNC = 1; @@ -62,8 +64,7 @@ export function set_is_destroying_effect(value) { let queued_root_effects = []; let flush_count = 0; -/** @type {Effect[]} Stack of effects, dev only */ -let dev_effect_stack = []; + // Handle signal reactivity tree dependencies and reactions /** @type {null | Reaction} */ @@ -451,10 +452,6 @@ export function update_effect(effect) { var teardown = update_reaction(effect); effect.teardown = typeof teardown === 'function' ? teardown : null; effect.version = current_version; - - if (DEV) { - dev_effect_stack.push(effect); - } } catch (error) { handle_error(/** @type {Error} */ (error), effect, previous_component_context); } finally { @@ -471,20 +468,12 @@ function infinite_loop_guard() { if (flush_count > 1000) { flush_count = 0; if (DEV) { + let location = assignment_stack.pop(); + try { - e.effect_update_depth_exceeded(); - } catch (error) { - // stack is garbage, ignore. Instead add a console.error message. - define_property(error, 'stack', { - value: '' - }); - // eslint-disable-next-line no-console - console.error( - 'Last ten effects were: ', - dev_effect_stack.slice(-10).map((d) => d.fn) - ); - dev_effect_stack = []; - throw error; + e.effect_update_depth_exceeded(location && sanitize_location(location)); + } finally { + assignment_stack.length = 0; } } else { e.effect_update_depth_exceeded(); @@ -560,16 +549,13 @@ function flush_queued_effects(effects) { function process_deferred() { is_micro_task_queued = false; - if (flush_count > 1001) { - return; - } const previous_queued_root_effects = queued_root_effects; queued_root_effects = []; flush_queued_root_effects(previous_queued_root_effects); if (!is_micro_task_queued) { flush_count = 0; if (DEV) { - dev_effect_stack = []; + assignment_stack.length = 0; } } } @@ -701,7 +687,7 @@ export function flush_sync(fn) { flush_count = 0; if (DEV) { - dev_effect_stack = []; + assignment_stack.length = 0; } return result;