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] Improve typescript types + minor fixes #1963

Merged
merged 9 commits into from
Sep 12, 2024
Merged

Conversation

pedroborges
Copy link
Collaborator

Reopening #1881 originally created by @jamesst20.

@pedroborges pedroborges added the svelte Related to the svelte adapter label Sep 6, 2024
@jamesst20
Copy link
Contributor

jamesst20 commented Sep 6, 2024

Hey @pedroborges

If you ever reopen the minimal upgrade for Svelte 5, I found a bug this week where I noticed SSR is no longer working with newer build of Svelte 5 starting from next.179+.

You may want to apply this as well

jamesst20@c8493ab

This is because Svelte 5 uses anchors (comments) and the div id=app is passed into the render(...) function of Svelte server and it makes anchors outside the div. So when it comes to hydrate the div it completely crash

@pedroborges
Copy link
Collaborator Author

Thanks for letting me know @jamesst20, I'll check it out. Really appreciate your work on these PRs 🙇‍♂️

@joetannenbaum
Copy link
Contributor

This all looks good, thank you both for all of the work you put into this.

When I fire up the Svelte playground I'm getting a bunch of warnings in the console, I just want to make sure that they aren't a by-product of this PR. Any insight here?

CleanShot 2024-09-11 at 16 46 08@2x

@pedroborges
Copy link
Collaborator Author

pedroborges commented Sep 11, 2024

When I fire up the Svelte playground I'm getting a bunch of warnings in the console, I just want to make sure that they aren't a by-product of this PR. Any insight here?

@joetannenbaum the adapter passes all props from server to every layout and page components. Since each component only declares the props it needs Svelte warns you about the extra ones. You only see these warnings in development so we are good 👍

Copy link
Contributor

@joetannenbaum joetannenbaum left a comment

Choose a reason for hiding this comment

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

This all looks good to me, thank you both for all of your work on it!

@pedroborges pedroborges merged commit f34b707 into master Sep 12, 2024
12 checks passed
@pedroborges pedroborges deleted the new-ts-fixes branch September 12, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
svelte Related to the svelte adapter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants