Skip to content

fix: prevent fatal error after 'reset_elements' has been triggered #13654

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

Closed
wants to merge 1 commit into from

Conversation

signalize
Copy link

In development mode, the function 'reset_elements' is executed. This function sets the 'parent' to null, which breaks the 'pop_element' function. This fix solves this.

I ran into this problem when I executed the following code. I had placed the "{@render hello()}" in a div.
#12425 (comment)

Code to reproduce the problem:

<script>
    import {createRawSnippet, hydrate} from 'svelte';
    import {render} from 'svelte/server';
    import Child from './Child.svelte';

    let {browser} = $props();

    let count = $state(0);

    const hello = createRawSnippet((count) => ({
        render: () => `
 			<div>${browser ? '' : render(Child).body}</div>
 		`,
        setup(target) {
            hydrate(Child, {target})
        }
    }));
</script>

<div>
{@render hello()}
</div>

Copy link

changeset-bot bot commented Oct 17, 2024

⚠️ No Changeset found

Latest commit: 8f50273

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@paoloricciuti
Copy link
Member

Hi, this is not a bug at all...the code that you mention assumes you will run the code during SSR (where browser will be false) and then during CSR.

The reason why your code is failing is because you are running with browser false in the client. If you just want to mount a component using createRawSnippet you should just do

<script>
    import { createRawSnippet, mount } from 'svelte';
    import Child from './Child.svelte';

    let count = $state(0);

    const hello = createRawSnippet((count) => ({
        render: () => `<div></div>`,
        setup(target) {
            mount(Child, {target})
        }
    }));
</script>

<div>
    {@render hello()}
</div>

Here's a REPL showing this.

If you are indeed building a framework with SSR and you want to SSR your component and then hydrate it you can use the original setup but you have to make sure that you will pass browser false on the server and browser true on the client so that svelte can create the html for you to hydrate on the client.

Thanks for contributing anyway!

@signalize
Copy link
Author

Hi @paoloricciuti ,

Thanks for your answer. Your example works fine indeed. This is because no component is rendered 'server-side'. I have created a branch in which you can reproduce the problem I experience. See:
https://github.com/signalize/tests-raw-snippet-error

My pull request is, upon closer inspection, not correct. The value of parent is specifically checked for 'null' in several places. I have made an adjustment that you can see here.

main...signalize:svelte:main

@paoloricciuti
Copy link
Member

Hi @paoloricciuti ,

Thanks for your answer. Your example works fine indeed. This is because no component is rendered 'server-side'. I have created a branch in which you can reproduce the problem I experience. See: https://github.com/signalize/tests-raw-snippet-error

My pull request is, upon closer inspection, not correct. The value of parent is specifically checked for 'null' in several places. I have made an adjustment that you can see here.

main...signalize:svelte:main

Hey took a look at your repo...i have a PR ready for it (not sure if it's the right fix)

@paoloricciuti
Copy link
Member

@signalize fixed it 🤟🏻

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 this pull request may close these issues.

2 participants