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

No error for @render undefined if snippet function is state #12999

Closed
rChaoz opened this issue Aug 23, 2024 · 16 comments · Fixed by #13083
Closed

No error for @render undefined if snippet function is state #12999

rChaoz opened this issue Aug 23, 2024 · 16 comments · Fixed by #13083
Assignees
Labels
Milestone

Comments

@rChaoz
Copy link
Contributor

rChaoz commented Aug 23, 2024

Describe the bug

Currently, @render has no problem if it the snippet function is undefined if it's a state/prop, which is weird since it is a function call. Considering the {@render func?.() syntax is supported, it should throw.

Case 1:

<script>
  let func = $state()
  $effect(() => {
    console.log(func === undefined) // true
  });
</script>

This doesn't throw: {@render func()}

Case 2:

<script>
  let func
  console.log(func === undefined) // true
</script>

This is an error: {@render func()}

Severity

annoyance

@rChaoz rChaoz changed the title No error for @render undefined if snippet is state No error for @render undefined if snippet function is state Aug 23, 2024
@rChaoz
Copy link
Contributor Author

rChaoz commented Aug 29, 2024

Is there a chance this can get looked at / added to the milestone? Fixing this would be a breaking change.

@paoloricciuti
Copy link
Member

To me it feels like we should do the opposite: just render nothing if func is undefined even if it's not state.

@trueadm trueadm added this to the 5.0 milestone Aug 30, 2024
@trueadm trueadm added the bug label Aug 30, 2024
@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

This is a bug, undefined or null should render nothing. Or it should always throw an exception.

@dummdidumm
Copy link
Member

Should it? Shouldn't we error if it's called but is undefined, similar to how you get a JS error when invoking undefined as a function?

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Then case 1 above should also throw an exception? Either way it's a bug.

@paoloricciuti
Copy link
Member

How do we want to fix it? I already have a fix locally in case we want to always render nothing...throwing should also be pretty straightforward

@Conduitry
Copy link
Member

I'm torn. I know in other situations we are erring on the side of rendering nothing. I don't know whether we want to say that we should error here because the syntax looks like a function invocation.

In a similar vein, if Foo is undefined, then <svelte:component this={Foo}/> renders nothing (as does <svelte:element this={Foo}/>), while <Foo/> is a runtime error. I don't know. We have varying behavior here, but all of these things feel like the same sort of thing to me as rendering a snippet, regardless of whether it's a function-like syntax.

@dummdidumm
Copy link
Member

For me it's more about the look - it looks like a function call, and a function call would error if you'd call a falsy value.

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Agreed that we should not allow state of undefined to render nothing, it should also throw an exception.

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Turns out that making it a runtime error for state is a bit tricker than expected due to how we compile the dynamic logic out.

@render optional?.()

and also

@render optional.()

Becomes this when compiled:

$.snippet(node_7, () => $$props.optional)

So I'm actually leaning towards all nullish entries being rendered as empty instead.

@paoloricciuti
Copy link
Member

Turns out that making it a runtime error for state is a bit tricker than expected due to how we compile the dynamic logic out.

@render optional?.()

Becomes

$.snippet(node_7, () => $$props.optional)

So I'm actually leaning towards all nullish entries being rendered as empty instead.

Can't the snippet function throw an error if it's undefined?

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Turns out that making it a runtime error for state is a bit tricker than expected due to how we compile the dynamic logic out.

@render optional?.()

Becomes

$.snippet(node_7, () => $$props.optional)

So I'm actually leaning towards all nullish entries being rendered as empty instead.

Can't the snippet function throw an error if it's undefined?

That's my point, optional?.() and optional() both compile to exactly the same thing.

@paoloricciuti
Copy link
Member

That's my point, optional?.() and optional() both compile to exactly the same thing.

Oh gotcha...I mean I'm perfectly fine to render empty but we could also add a parameter to snippet passing true or false based on if the function call is with optional or not?

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

What about?

@render optional?.foo?.()

or

@render optional?.foo()

What is the argument for those cases?

@Rich-Harris
Copy link
Member

That's my point, optional?.() and optional() both compile to exactly the same thing.

Surely that's the thing we should be fixing?

@trueadm
Copy link
Contributor

trueadm commented Aug 30, 2024

Roger that.

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.

6 participants