Checkbox with 1-way binding and change results in a weird state

[koffeinfrei]

When using a checkbox with 1-way binding and the change event the state of the checkbox doesn't match the UI

The following example visualizes the issue when clicking the checkbox

checkbox.mp4

{checked}
<Checkbox checked={checked} on:change={change} />

<script lang="ts">
  import { Checkbox } from 'carbon-components-svelte'

  let checked = false

  const change = event => {
   checked = event.target.checked
  }
</script>

The problem seems to be in the order of the on:change declarations in the Checkbox component.
Here is the diff that solved my problem:

diff --git a/node_modules/carbon-components-svelte/src/Checkbox/Checkbox.svelte b/node_modules/carbon-components-svelte/src/Checkbox/Checkbox.svelte
index bf2a00a..4c75280 100644
--- a/node_modules/carbon-components-svelte/src/Checkbox/Checkbox.svelte
+++ b/node_modules/carbon-components-svelte/src/Checkbox/Checkbox.svelte
@@ -80,10 +80,10 @@
       name="{name}"
       readonly="{readonly}"
       class:bx--checkbox="{true}"
-      on:change
       on:change="{() => {
         checked = !checked;
       }}"
+      on:change
       on:blur
     />
     <label for="{id}" title="{title}" class:bx--checkbox-label="{true}">

I used patch-package to patch carbon-components-svelte@0.44.7 for the project I'm working on.
This issue body was partially generated by patch-package.

[metonym]

I'm unable to repro when putting the code in the Svelte REPL: https://svelte.dev/repl/6f48c872218b49d89ab115e8e9b09e97?version=3.44.3

It's using the latest carbon-components-svelte version 0.51.1

Am I missing anything?

[koffeinfrei]

It took me some trials to figure out the combination of the versions that fixed the error. With the following patch (edited for brevity) the issue disappeared, meaning it was somewhere in the combination of an older carbon-components and/or carbon-components-svelte.

diff --git a/package.json b/package.json
index f5a49d64..866d99af 100644
--- a/clients/crm/package.json
+++ b/clients/crm/package.json
@@ -21,8 +21,9 @@
-    "carbon-components": "^10.29.0",
-    "carbon-components-svelte": "^0.44.5",
+    "carbon-components": "^10.50.0",
+    "carbon-components-svelte": "^0.51.1",
+    "carbon-icons-svelte": "^10.44.3",

[metonym]

Curious – why do you have carbon-components installed? Are you using SCSS for styling?

[koffeinfrei]

The issue is still present after all. Visually everything seems to match, but the internal state of the Checkbox component is still off. The mismatch becomes visible when using true as the initial value. I created an updated REPL that includes a copy of the current Checkbox component with an output statement: https://svelte.dev/repl/e04844b88c0e4a1187ba16823816e317?version=3.44.3

[koffeinfrei]

@metonym Any thoughts on this? Can you confirm the bug? Happy to provide a PR if so.

[metonym]

@koffeinfrei Just throwing it out there that I'm not sure if "on:change" should even be forwarded. "on:check" should be the event to listen for [1]. Regardless, I'm happy to review a PR that would fix the behavior.

[1] on:check: https://svelte.dev/repl/d194d091568a4cde9b236d39e9938891?version=3.44.3

[koffeinfrei]

I'm not sure if "on:change" should even be forwarded.

Do you mean because on:check should be favored? This is a basic native event of all input fields, so I think it should be supported.

"on:check" should be the event to listen for [1]

The issue with that event is that it doesn't include the field's name, only the checked value.
EDIT: I just remembered that the main issue was the reactivity of checked which led to an infinite loop when updating the property. I can't reproduce it in the REPL though. I'll check and let you know.

I'm happy to review a PR that would fix the behavior.

👉 #1013

[metonym]

Fixed in v0.52.0

Thanks for getting to the bottom of this!

[metonym]

Just to close the loop on the discussion:

Do you mean because on:check should be favored?

This is my personal opinion: my understanding of the "Svelte way" is to be as concise as possible.

This means using two-way binding instead of relying on dispatched events to manage state (like in React). However, I understand that it may be simpler sometimes to just listen to an event when you do not need to manage state (like <Toggle on:toggle /> for example).

This is a basic native event of all input fields, so I think it should be supported.

True.

[koffeinfrei]

Thanks! That was fast! 🙌

[gregorw]

👍