From 2070c8a16601d8e7ad73410661dfd58b85d88c36 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 16 Oct 2024 14:02:12 +0100 Subject: [PATCH] breaking: disallow state mutations in logic block expression (#13625) --- .changeset/eighty-dryers-pretend.md | 5 +++++ .../98-reference/.generated/client-errors.md | 2 +- .../svelte/messages/client-errors/errors.md | 2 +- .../src/internal/client/dom/blocks/each.js | 6 +++--- packages/svelte/src/internal/client/errors.js | 4 ++-- .../src/internal/client/reactivity/sources.js | 15 +++++++++++++-- .../samples/side-effect-each/_config.js | 17 +++++++++++++++++ .../samples/side-effect-each/main.svelte | 9 +++++++++ 8 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 .changeset/eighty-dryers-pretend.md create mode 100644 packages/svelte/tests/runtime-runes/samples/side-effect-each/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/side-effect-each/main.svelte diff --git a/.changeset/eighty-dryers-pretend.md b/.changeset/eighty-dryers-pretend.md new file mode 100644 index 0000000000..3416fd615a --- /dev/null +++ b/.changeset/eighty-dryers-pretend.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: disallow state mutations in logic block expression diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index 52580f0300..4e7016b337 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/documentation/docs/98-reference/.generated/client-errors.md @@ -125,5 +125,5 @@ Reading state that was created inside the same derived is forbidden. Consider us ### state_unsafe_mutation ``` -Updating state inside a derived is forbidden. If the value should not be reactive, declare it without `$state` +Updating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without `$state` ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 9bd7e8a654..570a384d7c 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -82,4 +82,4 @@ ## state_unsafe_mutation -> Updating state inside a derived is forbidden. If the value should not be reactive, declare it without `$state` +> Updating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without `$state` diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 67cb8df34e..e10aed3867 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -31,7 +31,7 @@ import { pause_effect, resume_effect } from '../../reactivity/effects.js'; -import { source, mutable_source, set } from '../../reactivity/sources.js'; +import { source, mutable_source, internal_set } from '../../reactivity/sources.js'; import { array_from, is_array } from '../../../shared/utils.js'; import { INERT } from '../../constants.js'; import { queue_micro_task } from '../task.js'; @@ -463,11 +463,11 @@ function reconcile(array, state, anchor, render_fn, flags, get_key) { */ function update_item(item, value, index, type) { if ((type & EACH_ITEM_REACTIVE) !== 0) { - set(item.v, value); + internal_set(item.v, value); } if ((type & EACH_INDEX_REACTIVE) !== 0) { - set(/** @type {Value} */ (item.i), index); + internal_set(/** @type {Value} */ (item.i), index); } else { item.i = index; } diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 26d38f0aba..a0f33fb784 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -343,12 +343,12 @@ export function state_unsafe_local_read() { } /** - * Updating state inside a derived is forbidden. If the value should not be reactive, declare it without `$state` + * Updating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without `$state` * @returns {never} */ export function state_unsafe_mutation() { if (DEV) { - const error = new Error(`state_unsafe_mutation\nUpdating state inside a derived is forbidden. If the value should not be reactive, declare it without \`$state\``); + const error = new Error(`state_unsafe_mutation\nUpdating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without \`$state\``); error.name = 'Svelte error'; throw error; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index b7b94afd5d..bb97ea34c8 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -28,7 +28,8 @@ import { BRANCH_EFFECT, INSPECT_EFFECT, UNOWNED, - MAYBE_DIRTY + MAYBE_DIRTY, + BLOCK_EFFECT } from '../constants.js'; import * as e from '../errors.js'; @@ -136,7 +137,7 @@ export function set(source, value) { if ( active_reaction !== null && is_runes() && - (active_reaction.f & DERIVED) !== 0 && + (active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 && // If the source was created locally within the current derived, then // we allow the mutation. (derived_sources === null || !derived_sources.includes(source)) @@ -144,6 +145,16 @@ export function set(source, value) { e.state_unsafe_mutation(); } + return internal_set(source, value); +} + +/** + * @template V + * @param {Source} source + * @param {V} value + * @returns {V} + */ +export function internal_set(source, value) { if (!source.equals(value)) { source.v = value; source.version = increment_version(); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-each/_config.js b/packages/svelte/tests/runtime-runes/samples/side-effect-each/_config.js new file mode 100644 index 0000000000..b17c83849f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-each/_config.js @@ -0,0 +1,17 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + + test({ assert, target }) { + const button = target.querySelector('button'); + + assert.throws(() => { + button?.click(); + flushSync(); + }, /state_unsafe_mutation/); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-each/main.svelte b/packages/svelte/tests/runtime-runes/samples/side-effect-each/main.svelte new file mode 100644 index 0000000000..5821d6ebc1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-each/main.svelte @@ -0,0 +1,9 @@ + + + +{#each items.sort() as item (item)} +

{item}

+{/each} +