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

Updating to [email protected] (or later) throws some errors. #10188

Closed
1 task
mwc opened this issue Feb 22, 2024 · 8 comments · Fixed by #10224
Closed
1 task

Updating to [email protected] (or later) throws some errors. #10188

mwc opened this issue Feb 22, 2024 · 8 comments · Fixed by #10224
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) needs repro Issue needs a reproduction pkg: svelte Related to Svelte (scope)

Comments

@mwc
Copy link
Contributor

mwc commented Feb 22, 2024

Astro Info

astro: latest
svelte: [email protected]+(excluding next.55)

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

1、If the svelte version is later than 5.0.0-next.55, the following error will be thrown, the result of rendering is not as expected as the next-55.:
image
2、Most likely because the 5.0.0-next.56 updated hydrate.
3. Not sure if this issue should be fixed by astro or by svelte.

What's the expected result?

like [email protected]

Link to Minimal Reproducible Example

none

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Feb 22, 2024
@bluwy bluwy added pkg: svelte Related to Svelte (scope) - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Feb 22, 2024
@bluwy
Copy link
Member

bluwy commented Feb 22, 2024

Yeah it needs to be fixed in Astro: sveltejs/svelte#10581 (comment)

@matthewp
Copy link
Contributor

@mwc would you be willing to submit a PR that fixes this? Should work not just in the next tag, of course.

@matthewp matthewp added the needs repro Issue needs a reproduction label Feb 22, 2024
Copy link
Contributor

Hello @mwc. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@mwc
Copy link
Contributor Author

mwc commented Feb 23, 2024

@matthewp 👌👌👌 I'll give it a try.

@bluwy
Copy link
Member

bluwy commented Feb 23, 2024

@mwc Sorry for not mentioning, but I was already looking into it in the past hour 😅 If you want to try, I can let you do so though. I haven't got far yet.

@mwc
Copy link
Contributor Author

mwc commented Feb 23, 2024

@bluwy
i can provide some ideas:

import { hydrate, unmount } from 'svelte';  // --> here
import { add_snippet_symbol } from 'svelte/internal';

// Allow a slot to be rendered as a snippet (dev validation only)
const tagSlotAsSnippet = import.meta.env.DEV ? add_snippet_symbol : (s) => s;

export default (element) => {
	return async (Component, props, slotted) => {
		if (!element.hasAttribute('ssr')) return;

		let children = undefined;
		let $$slots = undefined;
		for (const [key, value] of Object.entries(slotted)) {
			if (key === 'default') {
				children = createSlotDefinition(key, value);
			} else {
				$$slots ??= {};
				$$slots[key] = createSlotDefinition(key, value);
			}
		}

                 // --> here
		const component = hydrate(Component, {
			target: element,
			props: {
				...props,
				children,
				$$slots,
			},
		});

                // --> and here
		element.addEventListener('astro:unmount', () => unmount(component), { once: true });
	};
};

... then it work.

I'm afraid it will take me longer than you, so it's better if you fix it.

@bluwy
Copy link
Member

bluwy commented Feb 23, 2024

Yeah, we also need to use mount or hydrate depending of if client:only is used. If client:only is used, it should use mount. You can check it like this:

const bootstrap = client !== 'only' ? hydrate : render;

And finally we need bump the peer dep too to next.56:

"svelte": "^4.0.0 || ^5.0.0-next.1"

And also test it manually. We don't have automated tests for Svelte 5 yet but that's fine.

@mwc
Copy link
Contributor Author

mwc commented Feb 24, 2024

thanks @bluwy

under your guidance, I have tried to submit a pr(#10224), but it seems that it has not passed all the checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) needs repro Issue needs a reproduction pkg: svelte Related to Svelte (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants