From d06174e4616bd6c461982c3b8dab4a0a2e1def79 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 6 Aug 2024 16:46:36 +0100 Subject: [PATCH] chore: add error for derived self referencing (#12746) * chore: add warning for derived self referencin * update build * address feedback * address feedback * build * messages shouldn't end with a period * simplify test * regenerate * newlines are free * no need to export this, we can move it closer to where it's used * fix double negative --------- Co-authored-by: Rich Harris --- .changeset/new-cooks-roll.md | 5 +++++ .../svelte/messages/client-errors/errors.md | 4 ++++ packages/svelte/src/internal/client/errors.js | 16 ++++++++++++++ .../internal/client/reactivity/deriveds.js | 21 +++++++++++++++++++ .../samples/derived-fn-recursive/_config.js | 21 +++++++++++++++++++ .../samples/derived-fn-recursive/main.svelte | 11 ++++++++++ 6 files changed, 78 insertions(+) create mode 100644 .changeset/new-cooks-roll.md create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/main.svelte diff --git a/.changeset/new-cooks-roll.md b/.changeset/new-cooks-roll.md new file mode 100644 index 0000000000..d13c52a799 --- /dev/null +++ b/.changeset/new-cooks-roll.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: add error for derived self referencing diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 442a9e7684..f0ca476ff1 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -18,6 +18,10 @@ > Attempted to instantiate %component% with `new %name%`, which is no longer valid in Svelte 5. If this component is not under your control, set the `compatibility.componentApi` compiler option to `4` to keep it working. See https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes for more information +## derived_references_self + +> A derived value cannot reference itself recursively + ## each_key_duplicate > Keyed each block has duplicate key at indexes %a% and %b% diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index e0d6b73c39..20a338fd5b 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -93,6 +93,22 @@ export function component_api_invalid_new(component, name) { } } +/** + * A derived value cannot reference itself recursively + * @returns {never} + */ +export function derived_references_self() { + if (DEV) { + const error = new Error(`derived_references_self\nA derived value cannot reference itself recursively`); + + error.name = 'Svelte error'; + throw error; + } else { + // TODO print a link to the documentation + throw new Error("derived_references_self"); + } +} + /** * Keyed each block has duplicate key `%value%` at indexes %a% and %b% * @param {string} a diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 2b9f87dfbf..2063b96cb0 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,4 +1,5 @@ /** @import { Derived } from '#client' */ +import { DEV } from 'esm-env'; import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js'; import { current_reaction, @@ -11,6 +12,7 @@ import { increment_version } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; +import * as e from '../errors.js'; export let updating_derived = false; @@ -79,17 +81,36 @@ function destroy_derived_children(derived) { } } +/** + * The currently updating deriveds, used to detect infinite recursion + * in dev mode and provide a nicer error than 'too much recursion' + * @type {Derived[]} + */ +let stack = []; + /** * @param {Derived} derived * @returns {void} */ export function update_derived(derived) { + if (DEV) { + if (stack.includes(derived)) { + e.derived_references_self(); + } + + stack.push(derived); + } + var previous_updating_derived = updating_derived; updating_derived = true; destroy_derived_children(derived); var value = update_reaction(derived); updating_derived = previous_updating_derived; + if (DEV) { + stack.pop(); + } + var status = (current_skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY diff --git a/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/_config.js new file mode 100644 index 0000000000..ae38cafd69 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/_config.js @@ -0,0 +1,21 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: `\n0`, + + mode: ['client'], + + test({ assert, target }) { + const btn = target.querySelector('button'); + + btn?.click(); + + assert.throws( + flushSync, + 'derived_references_self\nA derived value cannot reference itself recursively' + ); + + assert.htmlEqual(target.innerHTML, `\n0`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/main.svelte new file mode 100644 index 0000000000..4ccb5e2d47 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-fn-recursive/main.svelte @@ -0,0 +1,11 @@ + + + + +{even}