fix: allow for arrays in member access in directives

paoloricciuti commented 2 weeks ago

Svelte 5 rewrite

Closes #11467 ...this uses a slightly more advanced regex to determine if a part of a member access should use a literal or an identifier to access it.

I've also added the code to handle this absurd situation

<div use:obj.my-arr[0] />

i don't know if actually want to remove this and just ignore this case 😄

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

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

[x] 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
[x] Prefix your PR title with feat:, fix:, chore:, or docs:.
[x] This message body should clearly illustrate what problems it solves.
[x] Ideally, include a test that fails without this PR but passes with it.

Tests and linting

[x] Run the tests with pnpm test and lint the project with pnpm lint

changeset-bot[bot] commented 2 weeks ago

🦋 Changeset detected

Latest commit: df749d4f7880a346e669531327d1f3cfd84bdcc0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ------ | ----- | | svelte | Patch |

Not sure what this means? Click here to learn what changesets are.

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

Rich-Harris commented 1 week ago

This fails with e.g. x[0][0] and also x["y-z"]. We could fix those things, but since none of it works in Svelte 4 and it's pretty wacky code to write, I'm inclined to just leave it as-is — just use {@const ...}. I think supporting x.y-z in the first place was a mistake (it might predate {@const ...}).

paoloricciuti commented 1 week ago

This fails with e.g. x[0][0] and also x["y-z"]. We could fix those things, but since none of it works in Svelte 4 and it's pretty wacky code to write, I'm inclined to just leave it as-is — just use {@const ...}. I think supporting x.y-z in the first place was a mistake (it might predate {@const ...}).

Wait really? I remember specifically testing for those cases. Can you approve the deploy on vercel so I can take a look? I'm on my phone

paoloricciuti commented 1 week ago

Btw I agree that this kind of stuff is pretty un-useful

paoloricciuti commented 1 week ago

This fails with e.g. x[0][0] and also x["y-z"]. We could fix those things, but since none of it works in Svelte 4 and it's pretty wacky code to write, I'm inclined to just leave it as-is — just use {@const ...}. I think supporting x.y-z in the first place was a mistake (it might predate {@const ...}).

With this

<script>
    let arr = $state([[()=>{}]])
</script>

<button use:arr[0][0]>
    clicks
</button>

the generated code is

import * as $ from "svelte/internal/client";

var root = $.template(`<button>clicks</button>`);

export default function App($$anchor) {
  let arr = $.proxy([[() => {}]]);
  var button = root();

  $.action(button, ($$node) => arr[0][0]($$node));
  $.append($$anchor, button);
}

which is correct and doesn't errors out.

With this

<script>
    let x = $state({"y-z": ()=>{}})
</script>

<button use:x.y-z>
    clicks
</button>

the generated code is

import * as $ from "svelte/internal/client";

var root = $.template(`<button>clicks</button>`);

export default function App($$anchor) {
  let x = $.proxy({ "y-z": () => {} });
  var button = root();

  $.action(button, ($$node) => x["y-z"]($$node));
  $.append($$anchor, button);
}

which is also correct and doesn't errors out.

Were you referring to a different code?

You can't do this

<script>
    let x = $state({"y-z": ()=>{}})
</script>

<button use:x["y-z"]>
    clicks
</button>

but that's actually a parsing error in svelte. We could fix it but i'ts not worth imho

Rich-Harris commented 1 week ago

Huh, weird — I could have sworn it was failing for me locally with [0][0] but I can't seem to reproduce it now

paoloricciuti commented 1 week ago

Huh, weird — I could have sworn it was failing for me locally with [0][0] but I can't seem to reproduce it now

Classic old version of the compiler still in cache or yet to finish build? Sometimes it even happen between refreshes if roll-up is a tad slower.

I tested all it came to mind but if there's some case not covered I'm happy to add that

sveltejs / svelte