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

Svelte 5: #each index used as key causes error #9408

Closed
brunnerh opened this issue Nov 12, 2023 · 5 comments · Fixed by #9441
Closed

Svelte 5: #each index used as key causes error #9408

brunnerh opened this issue Nov 12, 2023 · 5 comments · Fixed by #9441
Milestone

Comments

@brunnerh
Copy link
Member

Describe the bug

Presumably because the key function only gets passed the item.
Works in v4 so probably shouldn't cause errors in v5, even if it doesn't really do anything.

(Found by Thiagolino8 on Discord.)

Reproduction

{#each [1] as item, i (i)}
	{item}
{/each}

REPL

Logs

No response

System Info

REPL

Severity

annoyance

@benmccann benmccann added this to the 5.x milestone Nov 12, 2023
@trueadm
Copy link
Contributor

trueadm commented Nov 13, 2023

Isn't this the same as an indexed array? Why key it at all?

@brunnerh
Copy link
Member Author

I know, it's redundant.
But also a bit more explicit.
In any case, I only reported the issue 🤷

dummdidumm pushed a commit that referenced this issue Nov 14, 2023
Fixes #9408. Ensures that if we have a key that matches the index, that we fallback to using an indexed each block.
@AgarwalPragy
Copy link

This is not fixed.

  1. REPL gives the error Cannot have duplicate keys in a keyed each: Keys at index 0 and 1 with value '2' are duplicates
  2. For some reason, it still gives me ReferenceError: i is not defined when I run it locally

@brunnerh
Copy link
Member Author

This is a dev mode only issue. The production build is exactly the same (the implemented fix turns it into an indexed each).

This is currently still falsely added in dev mode:

$.validate_each_keys(() => [1, 2, 3], (item) => {
  return $.get(i);
});

@trueadm
Copy link
Contributor

trueadm commented Nov 24, 2023

Fixed in #9641.

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

Successfully merging a pull request may close this issue.

4 participants