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

[Feature]: Awaited routeData for nested routes #1053

Closed
2 tasks done
emdede opened this issue Sep 7, 2023 · 2 comments
Closed
2 tasks done

[Feature]: Awaited routeData for nested routes #1053

emdede opened this issue Sep 7, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@emdede
Copy link
Contributor

emdede commented Sep 7, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

In light of a looming Solid 2.0 release and the fresh announcement of SolidStart's 2nd beta, I would like get this on the roadmap, if not already being considered.

There's a need for control in regards to the execution of routeData functions when they're dependent on their parent.

Per its name it's clear that anything nested is in some way related to its parent. This, however, is not adequately dealt with in the current state of things.

MO as of now:

  • SSR request comes in
  • go through all the routes' data functions and execute them

The problem that this approach comes with is that it disregards any dependency of a hierarchical route structure.
This can be dealt with on the micro level but 1) you have to dive in rather deep to understand Solid's approach to this and 2) it can be quite cumbersome to implement.

In the following I will present a few thoughts on how to achieve this, starting with exploring the idea where in a Solid 2.0 there might be awaitable resources and whether they'd be viable to tackle this problem.

AWAIT

export function routeData(args) {
  const user = await getUserFromReq(args.req)
  if (!user) throw redirect('/login')
  return user
}

A protected entrypoint that fetches the user - simplified. Proper way would be to wrap this in createServerData$

export function routeData(args) {
  return createRouteData(() => {
    const user = await args.data()
    return api.get('someResource', user)
  })
}

Now this could work on paper, but I don't see the reactivity system playing along with this. For this scenario to succeed there'd need to be a dependency tree for all the routeData functions, afaict. Meaning, gather all routeData fns from top to bottom first and execute them one after another, after await has returned a result. Some problems, where I'm a bit out of my depth, would probably be cleanup and a proper context determination. Solvable probably, but this implementation still remains with a bit of boilerplate regardless. It also steers away from the core system in that it's a true waterfall execution and does not utilize Solid's primitives in any way.

GENERAL

I'm thinking there should be a global option to have this scenario triggered. Whether it be await or some other solution doesn't matter. An option where you can choose "I want my routeDatas to wait for their parents" or "I don't care about the parent's routeData - just execute them in one go is fine for me". With that then also the possibility to override this option for specific routes and their data.

In a scenario where I have awaitRouteDatas: true I would be able to just write something like this for the example above

export const routeData = (args) => createRouteData(() => api.get('someResource', args.data()))

And know that I have the resolved parent's routeData to work with.

WRAPPER

The way this can be achieved as of right now could also be integrated into the existing routeData implementations as a shortcut. For a nested route I have something like this

export function routeData(args) {
  return createRouteData(
    (user) => api.get('someResource', user),
    { key: args.data }
  )
}

This approach takes advantage of resources not firing with a falsy source signal. And at first glance this seems totally fine, but already introduces a significant amount of boilerplate to have to repeat this in every nested route and on top of that becomes increasingly ugly when you have other "real" key dependencies, like

export function routeData(args) {
 return createRouteData(
   ([_, id]) => id ? api.get('someResource', id) : undefined,
   { key: () => ['nestedRoute1', args.data() && args.params.aParam] }
 )
}

I have tried to build this wrapper myself but 1) the types needed are not exported 2) I dislike to fork out here since it basically means patching

What would be much easier is to be able to write something along the lines of

export function routeData(args) {
 return createRouteData(
   ([_, id]) => api.get('someResource', id),
   { 
     key: () => ['nestedRoute1', args.params.aParam],
     awaitParent: true // or a more general `dependsOn`
   }
 )
}

Coupled with a global (or per route-tree) option, where you wouldn't have to declare this every time, we're entering sweet territory.

Conclusion

The matter of properly dealing with nested routeData has thus far been neglected and I've seen this issue come up regularly in Solid's Discord server. This is not a fringe issue, it is needed for regular production applications.

Whether this issue should be opened here, in Solid's main repo or in the router repo, I am not sure. Please move it accordingly, if I made a wrong call. It just feels to me to be most prevalent in relation to SSR.

Middleware Thoughts:

  • With an extended middleware functionality, some of this could be alleviated. The main gripe with middleware currently is that the computation done is lost. I think if there's a solution where you can populate a ctx field (like tRPC does it) or data or whatever, that is accessible in routeData, that would help - but that would again bring up additional things to consider in regards to CSR
  • Even without a middleware approach, which mostly handles matters of authorization, there is a need to be able to reliably reference parent's routeData without having to implement safe guards in every single function - simply because of the nature of it being nested.

Edit: I didn't know how to properly split this into the template sections provided. I hope as is will be okay.

Edit 2: I realize that this a rather minor point in the grand scheme of things - that already has a solution. But it is something that presents a blocker for many SolidStart users as per my experience.

@emdede emdede added the enhancement New feature or request label Sep 7, 2023
@emdede
Copy link
Contributor Author

emdede commented Sep 7, 2023

Additional context depicting current usage in our own app:

root.tsx

...
<APIProvider>
  <Routes>
    <FileRoutes />
  </Routes>
</APIProvider>
(protected).tsx

export const routeData = (args) => {
  const { setAuthHeader } = useAPI()
  
  // returns Accessor<Session | undefined> or throws redirect
  const serverSession = getSession()
  
  return createRouteData(() => {
    (session) => {
      setAuthHeader(session.accessToken)
      return session
    },
    { key: serverSession }
  }
}
(protected)/[id].tsx

export const routeData = (args) => {
  const { request } = useAPI()
  
  return createRouteData(
    (id) => request('nestedResource', id),
    { key: () => args.data() && args.params.id }
  )
}

A bit simplified for terseness. The use-case here is that we generate an access token for a user that logs in via our SolidStart app. Without that access token, any API request would return 403. This is why it is crucial to await the protected layout's function to resolve successfully before trying to query the API. And this seems to be a rather common use-case.

@ryansolid
Copy link
Member

In setting up for SolidStart's next Beta Phase built on Nitro and Vinxi we are closing all PRs/Issues that will not be merged due to the system changing. If you feel your issue was closed by mistake. Feel free to re-open it after updating/testing against 0.4.x release. Thank you for your patience.

See #1139 for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants