fix: Still failing bind:group for nested data

[ciscoheat]

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Description

After the #10368 fix in next.45, I tried my code to see if #9947 was fixed, but unfortunately it doesn't work for a simpler case. Apologies for providing an edge case test in runtime-legacy/samples/binding-input-group-each-15, but not a more common one.

Currently, extract_all_identifiers_from_expression in ast.js won't extract both identifiers from $order.scoops and $order.flavours.

Attempted fix

My attempt of fixing this (by no means a perfect fix) is to make extract_all_identifiers_from_expression return all identifiers, and in 2-analyze/index.js use the binding or the identifier when detecting what group to bind to.

This fixes the test added in this PR, runtime-legacy/samples/binding-input-group-each-16 but makes another test fail, runtime-legacy/samples/binding-input-group-each-12. This makes me wonder if the combination of identifiers and each block to detect the group maybe isn't working properly.

In the case of each-16, the $order.flavours binding has no reference to its each block, which should make it "standalone", but still be fully referenced to distinguish it from $order.scoops.

On the other hand, each-12 references its each block, which references its parent, and so on, so maybe it should be referenced all the way up to pipelineOperations, but that's the limit of my knowledge on how to reference it properly.

[changeset-bot]

⚠️ No Changeset found

Latest commit: e056fa2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dummdidumm]

This will fail other parts of the code. And this would probably also yield a false-positive match for bind:group={group.a} and bind:group={group[a]}. We need to find a different way to compare these, possibly by exact string match. @tanhauhau how did this work exactly in Svelte 4?

[ciscoheat]

Just want to point out another possibly tricky case, in packages\svelte\tests\runtime-legacy\samples\binding-input-group-each-8\main.svelte, where the group binding is using the key of a higher each loop, not the one it's in:

{#each keys as key (key)}
  <h2>{key}</h2>
  <ul>
  {#each values as value (value)}
    <li>
      <label>
        <input type="checkbox" name={key} {value} bind:group={object[key]} /> {value}
      </label>
    </li>
  {/each}
  </ul>
{/each}

[ciscoheat]

My spontaneous thought is to use the bind:group expression itself as a key to the binding group object. What would be the problem with that?

[dummdidumm]

Thanks again for the test, I hope #10410 fixed this for good

[ciscoheat]

Thank you for taking the time to figure this out. :)