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

Fix-component-undefined-svelte-v5 #12102

Merged
merged 4 commits into from
Oct 5, 2024

Conversation

hermit99
Copy link
Contributor

@hermit99 hermit99 commented Oct 2, 2024

Changes

Small code enhancement, which is easy to understand.

  • Fix 'component is not defined' console error when unmounting svelte 5 component.

Console error: Screenshot 2024-10-01 110532

Testing

Defensive code added and tested locally.

Docs

Copy link

changeset-bot bot commented Oct 2, 2024

🦋 Changeset detected

Latest commit: ffa87ee

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

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

@github-actions github-actions bot added pkg: svelte Related to Svelte (scope) pkg: integration Related to any renderer integration (scope) labels Oct 2, 2024
@martrapp
Copy link
Member

martrapp commented Oct 5, 2024

Thanks @bluwy for pointing out!

@hermit99, thank you very much for providing this PR!
I'll have a look at #12116 and we might fix them both in the same way as the issues result from the same refactoring.

Copy link
Member

@martrapp martrapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hermit99, the suggestion would be to revert the file and only move the addEventlistener from line 44 up to line 42 at the end of the else-block. That would ensure that component is always defined when referenced and would also fix the issue that the listener is currently added too often

@hermit99
Copy link
Contributor Author

hermit99 commented Oct 5, 2024

Thanks @martrapp. Pardon me as I'm new to Astro's code base. Your suggestion makes more sense, and I really appreciate it ❤️

@martrapp
Copy link
Member

martrapp commented Oct 5, 2024

Thank you very much for taking care! 💜
The suggestion came from #12116 (comment)

@martrapp martrapp merged commit dcc1e89 into withastro:next Oct 5, 2024
12 of 13 checks passed
@hermit99 hermit99 deleted the fix-component-undefined-svelte-v5 branch October 5, 2024 10:04
martrapp added a commit that referenced this pull request Oct 5, 2024
* - fix: 'component is not defined' error when unmount svelte 5 component

* added changeset

* Moving unmount listener to where the component is defined.

* Update .changeset/eighty-ligers-punch.md

Co-authored-by: Martin Trapp <[email protected]>

---------

Co-authored-by: Martin Trapp <[email protected]>
martrapp added a commit that referenced this pull request Oct 5, 2024
* - fix: 'component is not defined' error when unmount svelte 5 component

* added changeset

* Moving unmount listener to where the component is defined.

* Update .changeset/eighty-ligers-punch.md



---------

Co-authored-by: Hermit <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope) pkg: svelte Related to Svelte (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants