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

In onblur handler: state_unsafe_mutation Updating state inside a derived or logic block expression is forbidden [svelte 5 issue] #13684

Closed
kyle-leonhard opened this issue Oct 19, 2024 · 13 comments · Fixed by #13737
Labels
Milestone

Comments

@kyle-leonhard
Copy link

kyle-leonhard commented Oct 19, 2024

Describe the bug

A state_unsafe_mutation error is triggered when an onblur handler changes a store value used in a $derived call.

I'm not really sure what the absolute minimal example is or the cause, I'm kind of a svelte newbie. I reproduced the error in a more contrived way in a repl attached.

This is triggered when upgrading from 5.0.0-next.267 to 5.0.0-next.269. I'm fairly sure it's triggered by #13625

Reproduction

Hover and then click one of the buttons in the repl

App.svelte:

<script>
  import {
    writable,
  } from "svelte/store";
  import Component2 from "./Component2.svelte";

  let highlighted = writable(null);
  let indices = $state([1,2,3,4,5]);

  function handleOnClick(index) {
    console.log(`Deleting ${index}`);
    delete indices[indices.findIndex((el) => el === index)];
  }
</script>

{#each indices as index}
	<Component2 {index} {highlighted} handleOnClick={() => {handleOnClick(index); }}></Component2>
{/each}

Component2.svelte:

<script>
  const { index, highlighted, handleOnClick } = $props();
	
  let elementt = $state();
  let isHovered = $derived($highlighted === index);

  function handleHighlight() {
    $highlighted = index;
  }
  function handleUnhighlight() {
    $highlighted = null;
  }
</script>

<div><button
  onmouseout={() => {
    handleUnhighlight();
  }}
  onmouseover={() => {
    handleHighlight();
  }}
  onblur={() => {
    console.log("onblur row");
    handleUnhighlight();
  }}
  onfocus={() => {
    console.log("onfocus row");
    handleHighlight();
  }}
  onclick={() => {
    handleOnClick();
  }}
  bind:this={elementt}>
	Row {index}, {isHovered}
</button></div>

Logs

Error: state_unsafe_mutation Updating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without `$state`
message:"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`"
stack:"Svelte error: state_unsafe_mutation\n" +
"Updating state inside a derived or logic block expression is forbidden. If the value should not be reactive, declare it without `$state`\n" +
" at state_unsafe_mutation (playground:output:244:18)\n" +
" at set (playground:output:339:4)\n" +
" at Array.eval (playground:output:3886:7)\n" +
" at Object.set (playground:output:4041:30)\n" +
" at store_set (playground:output:3905:9)\n" +
" at handleUnhighlight (playground:output:4119:4)\n" +
" at HTMLButtonElement.eval (playground:output:4135:4)\n" +
" at HTMLButtonElement.target_handler (playground:output:2675:20)\n" +
" at destroy_effect (playground:output:1408:10)\n" +
" at eval (playground:output:3352:5)"

System Info

System:
    OS: macOS 15.0.1
    CPU: (16) arm64 Apple M3 Max
    Memory: 16.74 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.9.0 - /opt/homebrew/bin/node
    npm: 10.8.3 - /opt/homebrew/bin/npm
    pnpm: 9.12.2 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 130.0.6723.59
    Safari: 18.0.1
  npmPackages:
    svelte: 5.0.0-next.269 => 5.0.0-next.269

Severity

blocking an upgrade

@kyle-leonhard
Copy link
Author

kyle-leonhard commented Oct 19, 2024

Likely triggered by #13625. fyi @trueadm

@huntabyte
Copy link
Member

huntabyte commented Oct 19, 2024

I'm getting something similar when using the class $state properties set by onblurcapture and other events.

Stacktrace for reference:
CleanShot 2024-10-18 at 21 12 20@2x

The #onblurcapture is being spread via a props object as an onblurcapture event to the HTMLDivElement, and it updates a class $state property in the same class as #onblurcapture method.

I'm having a difficult time creating a minimal repro per usual with Bits shenanigans but I know it was working before 268:
https://github.com/huntabyte/bits-ui/blob/e1715ce1178800825d0615e4b73048dde0f9fd67/packages/bits-ui/src/lib/bits/utilities/dismissible-layer/useDismissibleLayer.svelte.ts#L258-L260

@trueadm
Copy link
Contributor

trueadm commented Oct 19, 2024

I'm getting something similar when using the class $state properties set by onblurcapture and other events.

Stacktrace for reference: CleanShot 2024-10-18 at 21 12 20@2x

The #onblurcapture is being spread via a props object as an onblurcapture event to the HTMLDivElement, and it updates a class $state property in the same class as #onblurcapture method.

I'm having a difficult time creating a minimal repro per usual with Bits shenanigans but I know it was working before 268: https://github.com/huntabyte/bits-ui/blob/e1715ce1178800825d0615e4b73048dde0f9fd67/packages/bits-ui/src/lib/bits/utilities/dismissible-layer/useDismissibleLayer.svelte.ts#L258-L260

After digging into this, I found that it's is actually unrelated to that PR. Yes that PR surfaces the issue, but the actual issue is that when we remove the DOM elements, it triggers a synchronous blur event to fire and that happens inside a phase where the active reaction is still the block effect. I'll put up a fix for this.

This is annoying as Chrome is the only browser that incorrectly does this, see: https://chromestatus.com/feature/5128696823545856

@trueadm
Copy link
Contributor

trueadm commented Oct 19, 2024

Fixed with #13694

@trueadm trueadm closed this as completed Oct 19, 2024
@kyle-leonhard
Copy link
Author

kyle-leonhard commented Oct 19, 2024

Thanks for the super fast fix @trueadm !

The fix was effective in a few scenarios, both in my codebase and the attached repl. Unfortunately, I'm still seeing the issue in another scenario in my codebase. Briefly, a tr element is moved down a table via a keyboard shortcut handler which triggers the tr element's onblur handler which in turn writes to a store. The onfocus handler, which writes to the same store, is triggered immediately after.

I'll try to create a concise repro.

In the mean time, here's the stack trace, in case it offers a useful hint.

Uncaught Svelte error: state_unsafe_mutation
Updating state inside a derived or a template expression is forbidden. If the value should not be reactive, declare it without `$state`
    at state_unsafe_mutation (http://localhost:5173/node_modules/.vite/deps/chunk-CTGGALPS.js?v=73ab23eb:244:19)
    at set (http://localhost:5173/node_modules/.vite/deps/chunk-CTGGALPS.js?v=73ab23eb:2327:5)
    at Array.<anonymous> (http://localhost:5173/node_modules/.vite/deps/chunk-FJXO5MA5.js?v=73ab23eb:2913:11)
    at Object.set (http://localhost:5173/node_modules/.vite/deps/chunk-PQJYJW4C.js?v=73ab23eb:31:35)
    at Module.store_set (http://localhost:5173/node_modules/.vite/deps/chunk-FJXO5MA5.js?v=73ab23eb:2930:9)
    at HTMLTableRowElement.handleUnhighlight [as __mouseout] (http://localhost:5173/src/lib/something.svelte:327:5)
    at HTMLTableRowElement.target_handler (http://localhost:5173/node_modules/.vite/deps/chunk-26EVQ2H2.js?v=73ab23eb:40:22)
    at move (http://localhost:5173/node_modules/.vite/deps/chunk-FJXO5MA5.js?v=73ab23eb:950:10)
    at reconcile (http://localhost:5173/node_modules/.vite/deps/chunk-FJXO5MA5.js?v=73ab23eb:822:11)
    at http://localhost:5173/node_modules/.vite/deps/chunk-FJXO5MA5.js?v=73ab23eb:710:7
    ```

@dummdidumm dummdidumm reopened this Oct 19, 2024
@trueadm
Copy link
Contributor

trueadm commented Oct 19, 2024

@kyle-leonhard Looks like Svelte is moving the DOM element using element.before(target) in the each block, and somehow that invokes HTMLTableRowElement.target_handler. What is that handler hooked up to event wise? It sounds like another whack case that Chromium is doing, so if you could also confirm what browsers this affects, that would be great too.

I tried replicating it with a mouseout event, but that didn't work.

@Azarattum
Copy link
Contributor

Azarattum commented Oct 20, 2024

I observe a similar issue but with onoutrostart events. REPL

@Azarattum
Copy link
Contributor

I observe a similar issue but with onoutrostart events. REPL

This was partially fixed with #13719. There is no error anymore. However the UI doesn't update when state changes during an out transition: REPL. I guess this might be intended, since at this point all the render effects have already been cleaned up. Would just like to confirm that this works as expected. It might feel weird from a user's perspective.

@kyle-leonhard
Copy link
Author

@kyle-leonhard Looks like Svelte is moving the DOM element using element.before(target) in the each block, and somehow that invokes HTMLTableRowElement.target_handler. What is that handler hooked up to event wise? It sounds like another whack case that Chromium is doing, so if you could also confirm what browsers this affects, that would be great too.

I tried replicating it with a mouseout event, but that didn't work.

@trueadm

I poked at my code a little. afaict, the error is triggered by an onblur event, which I was able to verify with a little additional debug logging, and the issue replicates with the onmouseover/onmouseout event handlers removed. Interestingly, it doesn't replicate with the onfocus handler removed which writes to the same store as the onblur handler though logs tell me the onfocus handler always runs after the onblur.

Maybe the code itself would be helpful! The row element is defined here (0) and the onblur handler is bound to the tr element here (1).

The issue seems to impact Chrome and Chromium but not Safari.

One more interesting thing that may or may not be helpful. You mentioned element.before(target) earlier. The issue seems to occur when I shift a tr element down below the subsequent row but not upwards in the table, if that makes sense.

If there's useful debugging I can add, I'm happy to do so.

@7nik
Copy link

7nik commented Oct 21, 2024

However the UI doesn't update when state changes during an out transition: REPL. I guess this might be intended, since at this point all the render effects have already been cleaned up. Would just like to confirm that this works as expected. It might feel weird from a user's perspective.

It is by design: #12138 (comment)

@trueadm
Copy link
Contributor

trueadm commented Oct 21, 2024

@kyle-leonhard Looks like Svelte is moving the DOM element using element.before(target) in the each block, and somehow that invokes HTMLTableRowElement.target_handler. What is that handler hooked up to event wise? It sounds like another whack case that Chromium is doing, so if you could also confirm what browsers this affects, that would be great too.
I tried replicating it with a mouseout event, but that didn't work.

@trueadm

I poked at my code a little. afaict, the error is triggered by an onblur event, which I was able to verify with a little additional debug logging, and the issue replicates with the onmouseover/onmouseout event handlers removed. Interestingly, it doesn't replicate with the onfocus handler removed which writes to the same store as the onblur handler though logs tell me the onfocus handler always runs after the onblur.

Maybe the code itself would be helpful! The row element is defined here (0) and the onblur handler is bound to the tr element here (1).

The issue seems to impact Chrome and Chromium but not Safari.

One more interesting thing that may or may not be helpful. You mentioned element.before(target) earlier. The issue seems to occur when I shift a tr element down below the subsequent row but not upwards in the table, if that makes sense.

If there's useful debugging I can add, I'm happy to do so.

Ah, maybe blur fires sync when moving the element then.

Update: I can replicate it now

@kyle-leonhard
Copy link
Author

kyle-leonhard commented Oct 21, 2024

Thanks @trueadm, appreciate it! I'll give it a shot when the next release hits.

@kyle-leonhard
Copy link
Author

My issue is addressed! Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants