zth/rescript-relay-router

RFC: Route params as variants/polyvariants

zth opened this issue · 2 comments

zth commented

I've come across a few cases where it would be useful to have path params potentially be polyvariants/variants rather than just strings. One example:

/organization/:slug/analytics/:analyticsSource

...where I'd like analyticsSource to be one of a few set values. Example in pseudo code:

/organization/:slug/analytics/:analyticsSource(google|facebook|linkedin|otherServices)

I'm wondering whether it would make sense to support this OOTB in the router. As an example, the pseudo code above could generate these types:

analyticsSource: [#google | #facebook | #linkedin | #otherService]

React router support

It seems as v5 of React Router supports this pattern out of the box, so it'd just be a matter of generating the correct types from our side, which should be fairly easy.

However, in React Router v6 (which I guess we'll want to move to soon) they've removed path-to-regexp because of its file size. I guess we haven't decided what we want to do here, for all I care we could just vendor whatever we need/want that makes sense in our setup, and forget about React router as a concept. I'm all in favor of not allowing any regexps outside of very simple ones like the "or" above.

@Kingdutch what do you think?

Interesting! I’d prefer to follow React Router in this where possible, I think I read that they also removed Path2Regexp as a performance optimisation.

I do think what you’re proposing is sensible. I’d expect the router to give a 404 on any value that doesn’t match?

If my above (404) assumption is true then this is very similar to just registering a reusable route handler a couple of times with a different value per route. Given that we already have a build step we could also use that to just do that behind the scenes and compile this down to multiple static routes with a different set argument that’s a variant.

My preference would be a variant for non-optional parameters and a polyvariant for optional ones I think. To properly force handling the different cases throughout code and to let the ReScript compiler help update code for changing options.

zth commented

Interesting! I’d prefer to follow React Router in this where possible, I think I read that they also removed Path2Regexp as a performance optimisation.

I agree, let's follow React Router.

I do think what you’re proposing is sensible. I’d expect the router to give a 404 on any value that doesn’t match?

Yup! That route segment just wouldn't match.

If my above (404) assumption is true then this is very similar to just registering a reusable route handler a couple of times with a different value per route. Given that we already have a build step we could also use that to just do that behind the scenes and compile this down to multiple static routes with a different set argument that’s a variant.

Yes, exactly, that's how we can do it when we move to React Router v6. Just handle that syntax ourselves and compile it to actual route match entries for each given config.

Do we have optional path params, I don't think we do, right?

One question is whether to implement this now, while we're still on v5, and then just spend a bit of additional time when going to v6 since it'll require more changes. I think I'm in favor of that approach for 2 reasons:

  1. I don't want to move to v6 now, I want to wait a bit with that. Bigger fish to fry etc
  2. I'm interested in testing how this feels, and a quick implementation might be good for just testing things out

What do you think?