fix(multi-select): avoid cyclic dependency for Svelte 5 compatibility

[malinowskip]

Fixes #1986

Here’s an attempt to fix the issues in the MultiSelect component that caused incompatibility with Svelte 5 by creating infinite loops (#1986). It seems like the core issue was the circular dependency between the items and sortedItems variables:

items assigned tosortedItems in an afterUpdate block:

carbon-components-svelte/src/MultiSelect/MultiSelect.svelte

Line 261 in ac38e9d

items = sortedItems;

sortedItems depends on items:

  $: sortedItems = items.map((item) => ({
    ...item,
    checked: selectedIds.includes(item.id),
  }));

The commit removes the sortedItems variable, which served an intermediate role, and changes are made to items directly.

[malinowskip]

Inside afterUpdate, this block would cause an infinite loop.

[malinowskip]

tick ensures that the variables are instantiated when the sorting happens.

[metonym]

Thanks for taking the time to get this going. You're spot on in identifying the root issue of the infinite update loop in Svelte 5. There's a cyclical items <> sortedItems dependency in afterUpdate.

However, one thing I noticed is that this current change does not support initially set selectedIds.

<MultiSelect
  selectedIds={["0", "1"]}
  titleText="Contact"
  label="Select contact methods..."
  items={[
    { id: "0", text: "Slack" },
    { id: "1", text: "Email" },
    { id: "2", text: "Fax" },
  ]}
/>

[metonym]

I've pushed a 6fbedab that does the following:

• Remove the cyclical dependency in afterUpdate
• Consolidate around sortedItems internally to avoid the cyclical dependency

I've so far tested with both Svelte 4 and Svelte 5.

[malinowskip]

Thank for reviewing and catching the selectedIds issue. I think that your solution introduces one breaking change, though: the sorting logic doesn’t update the items variable, so the new sort order is not propagated to the parent (if items is bound reactively).

[brunnerh]

Arguably that was a bug; the property was not marked as being reactive and who really wants this?

[malinowskip]

It is marked as reactive in the docs (https://svelte.carbondesignsystem.com/components/MultiSelect#props), though, as you say, maybe it’s not useful for this prop to be reactive.

[brunnerh]

Right, I forgot that the tags are generated based on usage, so they don't appear in the source code JSDoc.

[metonym]

Arguably that was a bug; the property was not marked as being reactive and who really wants this?

Nice catch – MultiSelect items should not be reactive, and that was identified via this cyclic dependency fix. Generated docs need to be updated to reflect this – created #2042

[metonym]

Released in v0.86.0