solidjs / solid-router

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

Context providers inside `<Routes>` don't provide context values to routed content #273

Closed mathieuprog closed 8 months ago

mathieuprog commented 1 year ago

Describe the bug

This works:

<FooProvider>
  <Routes>
    <Route path=... element=... /> // the elements have access to context values
  </Routes>
</FooProvider>

This doesn't:

<Routes>
  <FooProvider>
    <Route path=... element=... /> // the elements don't have access to context values
  </FooProvider>
</Routes>

Use case: I want to wrap sets of routed content by specific context providers, as such:

<Routes>
  <I18nProvider namespaces={['auth']}>
    <AuthRoutes />
  </I18nProvider>

  <I18nProvider namespaces={['finance']}>
    <FinanceRoutes />
  </I18nProvider>

  <I18nProvider namespaces={['calendar']}>
    <CalendarRoutes />
  </I18nProvider>
</Routes>

Components such <AuthRoutes> contain something like:

<>
  <Route path=... element=... />
  <Route path=... element=... />
  <Route path=... element=... />
</>

As the above doesn't work, I have to wrap each individual Route by the context provider.

Platform

Brendan-csel commented 1 year ago

Writing this without testing so forgive me if not perfect :)

Does this work?

  <I18nProvider namespaces={['auth']}>
    <Routes>
      <AuthRoutes />
    </Routes>
  </I18nProvider>

  <I18nProvider namespaces={['finance']}>
    <Routes>
      <FinanceRoutes />
    </Routes>
  </I18nProvider>

  <I18nProvider namespaces={['calendar']}>
    <Routes>
      <CalendarRoutes />
    </Routes>
  </I18nProvider>

Also you may want to use <Route component={MyComponent}... instead of <Route element={<></>}... as I think the later might be running all the components immediately to pass into the route instead of creating them as you navigate - which is possibly also why you where having issues with where you placed the providers.

~EDIT: actually it might just be this last thing (using component instead of element) so maybe try that first.~ Nope - pretty sure <Route>s needs to always be direct children of <Routes> (or another <Route> if nesting).

mathieuprog commented 1 year ago

This is immensely valuable input, thank you!

So using component={} is more performant than element={} then? It is a little cumbersome on my side because I would have to create N new components to wrap the routed content, eg from element={<Content><OAuthFailure /></Content>} into component={OAuthFailureContent} (need to create a set of a new components such as OAuthFailureContent just to avoid element={}).

mathieuprog commented 1 year ago

I realized that the problem with defining multiple <Routes> as you have shown above, is that it will try to match a route inside every Routes collection, even if a route has already matched. In other words, when one Route matches inside a Routes, the subsequent Routes are not evaluated; however, with multiple Routes, if one Route matches within a Routes, the subsequent Routes inside the other Routes will still be evaluated. This causes 2 problems:

  1. inefficient
  2. I used to have a fallback route:
    <Route path="*" component={UnknownRouteErrorThrower} />

    in order to be notified in case a user accesses a route that doesn't exist. I can no longer use that fallback route. It will just render a blank page for unknown routes now.

The way I use the router feels like there is one level missing (which would allow to evaluate Route components inside Routes until there is a match, in which case Route components in subsequent Routes are not evaluated), like:

<RoutesGroup>
  <AuthRoutes />
  // if a route is matched in the AuthRoutes, do not try to match within subsequent <Routes>
  <FinanceRoutes />
  <CalendarRoutes />
  // fallback
  <Routes>
    <Route path="*" element={<div>error</div>} />
  </Routes>
<RoutesGroup>
ryansolid commented 1 year ago

Yeah the Routes definition is just JSON so it can't contain providers etc. It needs to be direct. This is very much by design.

Context in RouteData when nested doesn't work because it is hoisted. However if you only use Context in the components you could make grouped routes. Wrap them in route sections that either are a parent match or even empty path("") and then use the component there to wrap context around the outlet.

<Routes>
    <AuthRoutes />
    <FinanceRoutes />
    <CalendarRoutes />
</Routes>

function AuthRoutes = () => {
  return <Route path="" element={<I18nProvider namespaces={['auth']}><Outlet /></I18nProvider>}>
    <Route path=... element=... />
    <Route path=... element=... />
    <Route path=... element=... />
  </Route>
}
whoiscarlo commented 4 months ago

Yeah the Routes definition is just JSON so it can't contain providers etc. It needs to be direct. This is very much by design.

Context in RouteData when nested doesn't work because it is hoisted. However if you only use Context in the components you could make grouped routes. Wrap them in route sections that either are a parent match or even empty path("") and then use the component there to wrap context around the outlet.

<Routes>
    <AuthRoutes />
    <FinanceRoutes />
    <CalendarRoutes />
</Routes>

function AuthRoutes = () => {
  return <Route path="" element={<I18nProvider namespaces={['auth']}><Outlet /></I18nProvider>}>
    <Route path=... element=... />
    <Route path=... element=... />
    <Route path=... element=... />
  </Route>
}

@ryansolid Now that <Outlet /> component and the element attribute no longer exist, what the new solution for this problem?

whoiscarlo commented 4 months ago

I guess this is a solution?

const AuthComponent = (props) => {
  return (
    <I18nProvider namespaces={['auth']}>
      {props.children}
    </I18nProvider>
  )
}
function AuthRoutes = () => {
  return <Route path="" component={AuthComponent}>
    <Route path=... component=... />
    <Route path=... component=... />
    <Route path=... component=... />
  </Route>
}

It works but it feels wrong for some reason..