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: Snippets aren't deconflicted #12186

Closed
Rich-Harris opened this issue Jun 26, 2024 · 5 comments · Fixed by #12221
Closed

Svelte 5: Snippets aren't deconflicted #12186

Rich-Harris opened this issue Jun 26, 2024 · 5 comments · Fixed by #12221
Assignees
Labels
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the bug

It should be possible to have two snippets with the same name in different scopes. Right now, the second clobbers the first.

Reproduction

link

Logs

No response

System Info

next

Severity

annoyance

@Rich-Harris Rich-Harris added this to the 5.0 milestone Jun 26, 2024
@trueadm trueadm self-assigned this Jun 28, 2024
@trueadm
Copy link
Contributor

trueadm commented Jun 28, 2024

I originally thought to make the snippets const and then wrap their element scope in a block statement. This fixes this particular bug, but then they're no longer accessible in the <script> and the tests that test for that fail. I guess the only option is to rename the deconflicted variables instead.

@Rich-Harris
Copy link
Member Author

Snippets declared inside an element (or anything else) shouldn't be accessible in the <script>

@trueadm
Copy link
Contributor

trueadm commented Jun 28, 2024

Ah there were issues in my approach. Notably, we can't just wrap the init in a block statement, we also have to account for the after_update and update logic too. However, we can't really wrap them in the same block statement as they're designed to come much later on. Eh.

@dummdidumm
Copy link
Member

top level is function or var, nested ones are const?

@trueadm
Copy link
Contributor

trueadm commented Jun 28, 2024

Got it working locally, PR incoming.

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

Successfully merging a pull request may close this issue.

3 participants