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

track assignment locations for infinite loop reporting #13344

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
4 changes: 3 additions & 1 deletion packages/svelte/messages/client-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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)`
Expand All @@ -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'));
}
Expand All @@ -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;
}
};
}
Expand Down
9 changes: 9 additions & 0 deletions packages/svelte/src/internal/client/dev/assignment-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/** @type {string[]} */
export const assignment_stack = [];

/**
* @param {string} location
*/
export function track_assignment(location) {
assignment_stack.push(location);
}
7 changes: 7 additions & 0 deletions packages/svelte/src/internal/client/dev/utils.js
Original file line number Diff line number Diff line change
@@ -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');
}
5 changes: 2 additions & 3 deletions packages/svelte/src/internal/client/dom/blocks/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
}

/**
Expand Down
7 changes: 4 additions & 3 deletions packages/svelte/src/internal/client/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { FILENAME, HMR, NAMESPACE_SVG } 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';
Expand Down
34 changes: 10 additions & 24 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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} */
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -701,7 +687,7 @@ export function flush_sync(fn) {

flush_count = 0;
if (DEV) {
dev_effect_stack = [];
assignment_stack.length = 0;
}

return result;
Expand Down
Loading