solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.13k stars 143 forks source link

Cannot read properties of undefined (reading 'length') #272

Open mathieuprog opened 1 year ago

mathieuprog commented 1 year ago

Describe the bug

It took me forever to debug an error because the error message got lost by the router. Demonstration:

function throwError(): string {
  throw new Error('A meaningful error message');
}
<Route
  path={throwError()}
  element={<div>hello</div>}
/>

This leads to the following error:

Uncaught TypeError: Cannot read properties of undefined (reading 'length') at components.jsx:40:43 at dev.js:453:34 at untrack (dev.js:434:12) at Object.fn (dev.js:453:20) at runComputation (dev.js:696:22) at updateComputation (dev.js:679:3) at Object.readSignal (dev.js:612:67) at get when [as when] (components.jsx:64:25) at Show.createMemo.equals [as fn] (dev.js:1443:44) at runComputation (dev.js:696:22)

The original error message ('A meaningful error message') got lost.

Platform

Brendan-csel commented 1 year ago

I suspect it was something else giving you the Cannot read properties of undefined.

Check that the direct parent component of <Route> is either <Routes> or another <Route> (if nesting).

If you are still a having an issue, share a link to a simple reproduction and I can take a peek.

mathieuprog commented 1 year ago

@Brendan-csel I have added the following in the app (copy paste):

const App: VoidComponent = () => {
  function throwError(): string {
    throw new Error('A meaningful error message');
  }

  return (
    <>
      <Routes>
        <Route
          path={throwError()}
          element={<div>hello</div>}
        />
      </Routes>

Which outputs:

Uncaught TypeError: Cannot read properties of undefined (reading 'length') at components.jsx:40:43 at dev.js:453:34 ...

If you have an app with Solid router, may I first ask you to simply copy paste what is above, if it'd only require a copy paste for you? 🙏

If not, I will create a repo (it's just that it's not trivial with all the JS/TS configs).

Brendan-csel commented 1 year ago

If I don't have a wrapping <ErrorBoundary> I get this in the console...

image

If I have an ErrorBoundary that logs...

        <ErrorBoundary
          fallback={(err) => {
            console.error(err);
            return <div>{err}</div>;
          }}
        >
          <Routes>
            <Route path={throwError()} element={<div>hello</div>} />
          </Routes>
        </ErrorBoundary>

image

mathieuprog commented 1 year ago

I think in the last print screen, that first error log "A meaningful error message" is from the previous code without the error boundary. I think you might not have cleared your console? (hence having 3 logs)

You're right, it's the Error Boundary. But my Error Boundary doesn't log the error message "A meaningful error message". It's lost.

function fallback(e: any) {
  console.error(e);
  return <div>error</div>
}

render(
  () => (
    <ErrorBoundary fallback={fallback}>
      <Router>
        <App />
      </Router>
    </ErrorBoundary>
  ),
  document.getElementById('root') as HTMLElement
);

Could you make sure that you do have the error log "A meaningful error message" WITH the Error Boundary? In which case I create a repo to reproduce.

Brendan-csel commented 1 year ago

Yep - I get those 3 logs on page refresh. I might be using a slightly older version of the router though so it is probably worth you setting one up the same as you have there.

One other observation is my ErrorBoundary is inside Router and wraps Routes.

mathieuprog commented 1 year ago

Here is the repo: https://github.com/mathieuprog/solid-router-error-boundary

git clone git@github.com:mathieuprog/solid-router-error-boundary.git

npm i

npm run dev

Console:

index.tsx:18 TypeError: Cannot read properties of undefined (reading 'length') at components.jsx:40:43 at dev.js:453:34 at untrack (dev.js:434:12) at Object.fn (dev.js:453:20) at runComputation (dev.js:696:22) at updateComputation (dev.js:679:3) at Object.readSignal (dev.js:612:67) at get when [as when] (components.jsx:64:25) at Show.createMemo.equals [as fn] (dev.js:1443:44) at runComputation (dev.js:696:22)

Brendan-csel commented 1 year ago

Curiously if you delete the ErrorBoundary you see your message. I've run out of time to look further tonight. There are quite a few differences between your project and the (older) only I tested in - so not immediately sure why they are behaving differently. Sorry I can't be more help.

mathieuprog commented 1 year ago

With Solid 1.6.9 (my error message is NOT lost):

index.tsx:15 Error: A meaningful error message at throwError (App.tsx:7:11) at get path [as path] (App.tsx:13:18) at Object.get [as path] (dev.js:1323:41) at createRoutes (routing.js?v=b29f2bbc:78:29) at createBranches (routing.js?v=b29f2bbc:121:28) at Object.fn (components.jsx:36:37) at runComputation (dev.js:705:22) at updateComputation (dev.js:688:3) at createMemo (dev.js:244:10) at Routes (components.jsx:36:20) [...]

index.tsx:17 Uncaught TypeError: _tmpl$ is not a function at index.tsx:17:20 at fallback (index.tsx:20:7) at dev.js:1510:71 at untrack (dev.js:427:12) at ErrorBoundary.createMemo.name [as fn] (dev.js:1510:57) at runComputation (dev.js:705:22) at updateComputation (dev.js:688:3) at runTop (dev.js:791:7) at runQueue (dev.js:861:42) at completeUpdates (dev.js:817:84)

With Solid 1.7.0 (the error message IS lost):

TypeError: Cannot read properties of undefined (reading 'length') at components.jsx:63:39 at dev.js:453:34 at untrack (dev.js:434:12) at Object.fn (dev.js:453:20) at runComputation (dev.js:695:22) at updateComputation (dev.js:678:3) at Object.readSignal (dev.js:611:67) at get when [as when] (components.jsx:88:14) at Show.createMemo.equals [as fn] (dev.js:1420:44) at runComputation (dev.js:695:22)

cc @ryansolid

ryansolid commented 1 year ago

Yeah this is a Solid issue since 1.6.10. Odd that I can't seem to find the commit that actually breaks it as most of the commits are TS or SSR related. There is one commit that changes behavior but reapplying those changes or removing them I've had no luck getting into the other state. Errors have been something that we've been fiddling with the last couple releases.

This does cement the desired behavior on error handling propagation in general. Originally we'd error in a Memo and it would make it up to the Error Boundary and show it. This isn't great as it basically bails way to eagerly. Now the Memo gets in an errored state and then reads blank and causes a new Error.. What we want to happen is sort of the middle. For reading an Errored memo to throw its same error and propagate in a similar way up to effects but only to do so on read. Solid 2.0 will have such a system but I'm concerned that meddling here too much in 1.x just causes a different error.