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

[Bug?]: ErrorBoundary not working in nested (children) layouts #1677

Closed
2 tasks done
PawelJastrzebski opened this issue Nov 14, 2024 · 4 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@PawelJastrzebski
Copy link

PawelJastrzebski commented Nov 14, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

ErrorBoundary not catch error in children node.

export function PageLayout({ children }: { children?: JSXElement }) {

    return (
        <ErrorBoundary fallback={<div>Error PageLayout</div>} >
            {children}
        </ErrorBoundary>
    )
}

Expected behavior 🤔

should catch error like in this case: Pass by props.

export function PageLayoutWorks({ children }: { children: () => JSXElement }) {

    return (
        <ErrorBoundary fallback={<div>Error PageLayoutWorks</div>} >
            {children()}
        </ErrorBoundary>
    )
}

Steps to reproduce 🕹

Full Example: https://github.com/PawelJastrzebski/solidStart_error_boundry

Context 🔦

Catch errors on page Layout level.

I'm not sure if this supposed to work on the children level, or is it not working on children level by design?

Your environment 🌎

Node: v20.18.0
Npm: 10.8.2

OS: Ubuntu 24.04 noble
Kernel: x86_64 Linux 6.8.0-48-generic

"dependencies": {
  "@solidjs/meta": "^0.29.4",
  "@solidjs/router": "^0.15.0",
  "@solidjs/start": "^1.0.10",
  "solid-js": "^1.9.2",
  "vinxi": "^0.4.3"
}
@PawelJastrzebski PawelJastrzebski added the bug Something isn't working label Nov 14, 2024
@mdynnl
Copy link

mdynnl commented Nov 14, 2024

tldr; don't destructure
(I dislike this answer actually as it hides one of the most important concepts of how solid (jsx) actually works aka. deferred execution)

The latter works because children is passed as a function and further called inside jsx which the compiler turns it into a getter props object { get children() { return children() } } effectively deferring the execution inside the scope of ErrorBoundary.

And the former breaks because children passed in the same manner get immediately evaluated at the time of de-structuring right inside the function param ({ children }) when the component runs and the error stops the execution before ErrorBoundary isn't even called yet and hence fails to catch the error (which a simple console log or debugger would've uncovered)

https://playground.solidjs.com if you'd check the compiler output and the runtime behavior.

@PawelJastrzebski
Copy link
Author

Thank you for your answer @mdynnl

      <PageLayout >
           <CounterThrowOnMount />
      </PageLayout>

This case doesn't catch an error as well, even if error is throw on component mount.

I don’t mean to nitpick, but this behavior is a bit confusing to me. It would be great to have a longer explanation of edge cases in the SolidDocs. I think this could be really helpful for those of us who are less familiar with solid.

@mdynnl
Copy link

mdynnl commented Nov 15, 2024

The solution is to fix the PageLayout to something like

function PageLayout(props) {
  return <...>{props.children}</...>
}

as the de-structuring is the cause.

@PawelJastrzebski
Copy link
Author

PawelJastrzebski commented Nov 15, 2024

@mdynnl Thank you! You made my day.

Here is reference to documentation with explanation why restructuring props is not recommended. SolidDocs

I guess this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants