RFC: More flexible link creators

[zth]

We currently have type safe route creators. They're fantastic for generating links in a type safe way. However, they're not great for generating new URLs from existing ones.

Imagine I'm in a backoffice on /organization/123/members?search=Name&order=asc&orderBy=created. Switching any of the query params here is easy thanks to our Route.useQueryParams hook which let us spread the old params before setting new params, allowing us to just update the query params we want while retaining the rest. This hook also performs an actual update, it doesn't give me back a URL I can stick in a link.

Imagine I want to switch out what organization I'm on. I'll need to run Route.makeLink again and re-insert all query params by hand to the value they currently are. Depending on where those query params are controlled, it might be easy or hard to even access them where I'm creating my link.

Proposed API:

let {makeLink} = Route.useDynamicLink()

<RelayRouter.Link to_={makeLink(~params=currentParams => {...currentParams, organizationId: Some(newOrgId)}, ())

The difference to the current method exposed by Route.useQueryParams is that this would also allow setting any path params, and generate a full new link from that. If we leave it at just generating the actual URL, one could delegate all of the "tough" decisions around "should I update the URL or not?" to the user by letting them choose - stick it in a link, use the programmatic push/replace API (with potential shallow support in the future), etc.

This could be generated in addition to the current makeLink, which still would be the most used API.

[Kingdutch]

I think the idea here is really interesting. I'd be curious to figure out how this works together with data loading and prefetching.

I do think the current separation between path parameters and query params is an interesting one. If I look at my own usage then query params usually change when I update a GraphQL fragment (e.g. for pagination). While mixing the two allows the user more control over how they generate and structure their URLs, it may make it less predictable about when the router (pre)loads data vs when you still need to handle re-fetching yourself?

Can we map out different scenario's here (e.g. "changing param with constant query", "changing query with constant param", "changing neither", "changing both" etc.) and explain how we would expect the data requirements to change and what we expect the router to do (e.g. load data for changed params since it's a different route) and the user to do (refetch only a fragment since query params changed)?

[zth]

While mixing the two allows the user more control over how they generate and structure their URLs, it may make it less predictable about when the router (pre)loads data vs when you still need to handle re-fetching yourself?

Note that this is already how things are working today. makeLink mixes query params and path params freely today.

Let's start with the use cases we want to support.

Regular links

As in "link to route X with params Y". Typically <RelayRouter.Link to_=Routes.Some.Route.make(). Preloading or not is left to decide by whoever renders the link.

The behavior of <Link /> is another, but of course related, subject imo. We'll get to that too. But for these use cases, Link just work like it does today- it does a full navigation, with full preload evaluation etc.

Updating query params and bypassing preloading

This also exists and works today. This use case is for anything when you're doing something on a specific route, but want fine grained control over exactly how and what data is refetched/re-evaluated. Loading the next page, setting new filters in a search, revealing details, etc.

The important thing here is that you're effectively opting out of the router re-doing data fetching, which would affect the queries made for the route. What you're instead doing is you're refetching one or more fragments rendered, which sort of chops off that part of the UI with Relay and updates just that. The query still renders the same instance as it was inited with for the route, you're just updating a small part of it that it doesn't really care about (or will even know about unless it's asking for explicit data that's overlapping) since it's hidden behind a fragment.

Changing path params, preserving all (or some) of the current query params

This is something we don't support today, but that this RFC is about. One example use case is having a filter view or similar where you're working on resource X, denoted by an id as a path param in the URL, and you want to swap resource X to resource Y. Just like the RFC details, this is currently a very manual and potentially error prone process where you need to remember to pass around the current value of all params you care about.

<Link />

The behavior here is quite hard to nail imo. I can think of a couple of things I'd like <Link /> to do in a utopian scenario:

• Work like a regular link. This already works
• Allow for the same behavior as detailed in "Updating query params", as in bypass router update and just update the query params

Point 2) is interesting, but a bit difficult. Do you get what I'm after?

Random list of other things that pop into my mind:

• What about relative links? ../ for example. I can see the value of being able to link "one level up" without necessarily needing to detail what that level is. But things like../<new-id-here> makes little sense in our world since things are already type safe.
• Remembering the last value of parameters. I have a few use cases for this in my app where I need to remember what the last value of query param X was. This should probably live in user land though.

Can you think of more use cases? I'll try and do some additional thinking too.

[Kingdutch]

I can't come up with any additional use cases. I feel that the ../ should be easy if that is a separate nested route (since we know the hierarchy we know the parent). If it's not a separate nested route then it's not addressable so I maybe wouldn't support it?

I think one of the things I'd like to make sure we keep in mind is proper support for history and back 'n forth. Some interesting videos about this:

• The big back button quiz | HTTP 203
• The history API is dead. Long live the navigation API | HTTP 203

For the query parameters I wonder if, rather than making it easier to opt-out of the router we should make it easier to respond to changed query parameters (e.g. where we now have preload, make it possible to only prefetch the updated fragment).

I realised in this discussion that in my own implementation for navigation, by changing the query parameters myself and only fetching the fragment, I'm only fetching data when the user clicks. This is slower than the Router's default of fetching data when the user hovers (which leads to instant navigation after the fact). Now I actually have a bug in my application where I forgot to disable preloading, so my links are refetching the entire page again which means my fragment refetch query never hits the network, but I am overfetching. It'd be great if I could say "use this fragment query for this navigation, rather than the full page query" while preserving all the nifty preloading behaviour :D

Something that we talked about previously that may be related to all this would be parallel routing (my own concrete example is our Real-Time Chat, which is an "application" independent of the page the user is currently on; I'm currently just using some React state, but one of the open feature requests is deep links so I can actually open the chat to a specific conversation which is currently not yet possible).

[zth]

I can't come up with any additional use cases. I feel that the ../ should be easy if that is a separate nested route (since we know the hierarchy we know the parent). If it's not a separate nested route then it's not addressable so I maybe wouldn't support it?

Agreed. I think this might be something worth looking at later on, if there's still need.

I think one of the things I'd like to make sure we keep in mind is proper support for history and back 'n forth. Some interesting videos about this:

Yeah, this is of course very important. That's why it's important that you have full control over what ends up in history, but can control what the router does.

I realised in this discussion that in my own implementation for navigation, by changing the query parameters myself and only fetching the fragment, I'm only fetching data when the user clicks. This is slower than the Router's default of fetching data when the user hovers (which leads to instant navigation after the fact). Now I actually have a bug in my application where I forgot to disable preloading, so my links are refetching the entire page again which means my fragment refetch query never hits the network, but I am overfetching. It'd be great if I could say "use this fragment query for this navigation, rather than the full page query" while preserving all the nifty preloading behaviour :D

Fully agree, this should be the experience we go for - the same nice things (preloading etc) that regular links have, but a way to just "slice it off" and act independently of the router.

This is very crude, but captures what I have in mind:

<RelayRouter.Link to_=Routes.Whatever.Route.make(...config...) performNavigation={(params, _navigationType) => {
  // navigationType = Preload | Navigation
  startTransition(() => {
    let _ = refetch(~variables=WhateverFragment.makeRefetchVariables(....
   })
}

The naming is terrible, and the API isn't right (plus the type safety of 'params will require some thought), but the point is that setting performNavigation opts out of an actual navigation, and delegates what "navigate" means to the provided function. This means that the link will still get a valid href, and that'll work if JS is turned off/hydration hasn't happened yet, etc. However, if JS is active, performNavigation is what's run instead of pushing to the router. This lets the router use performNavigation to preload as well as do actual navigation. And startTransition gives you a bool to know whether the link is transitioning or not.

One thing it doesn't solve though is how to know if the transition is active because of a preload (don't want to show a loading indicator) or an actual navigation/action (want to show a loading indicator). One could solve it in user land by having two useTransition and deciding on them based on what navigationType is, but that feels a bit crude.

What do you think about something like that? The concept/gist of it, not the actual pseudo code written above.

[zth]

<RelayRouter.ShallowLink 
  link=Routes.Whatever.Route.makeShallowLink(~organizationSlug=org.slug, ...more config..., ()) 
  onNavigation={(params, _navigationType) => {.....

Another potential API that'll let us deal with type safety in an easier way.

Combining with the "dynamic routes" proposal:

let {makeShallowLink} = Routes.Whatever.Route.useDynamicRoute()

<RelayRouter.ShallowLink 
  link=makeShallowLink(~someQueryParam=someNewValue, ()) 
  onNavigation={(params, _navigationType) => {.....

makeShallowLink would only take query params here, so that would solve the "link to something with other path params is illegal".

[Kingdutch]

I think one of my personal thoughts was: why do we want to solve this in Link? Since we already need to declare our fragments, would it instead be possible to put the reloading logic in our Route_renderer file?

We could have a separate method for paramsChanged or something. Then the router could call that method to do preloading and prefetching and from the component perspective you could just treat it as a normal navigation, leveraging all the other options the router already provides in how you deal with loaders and have nesting.

[zth]

This is tricky 😃 I would say putting things in the route renderers for this would be an anti pattern, because that would force the route renderer to know about things it shouldn't care about - individual fragments somewhere in the hierarchy. The point of an API like this would be to let components slice up the graph and refetch whatever it needs, however it needs it, leveraging the URL as state, but without the main router data loading needing to know about it.

Do you follow what I say, and why it'd be an anti pattern? I can put together examples if not.

[Kingdutch]

Yeah I get it. I previously thought that the router was using fragments anyway, but it probably only knows about some top-level page component and a fragment may be buried many fragments deep.

With that said, if the fragment relies on query parameters then there is already some kind of coupling between the route renderer and the fragment right? Why is that not problematic? Could/should that be eliminated somehow through some indirection? 🤔

[zth]

Yes, I believe we've talked about that before, but the big mind shift here vs more traditional routing is that a route here shouldn't know about what data it loads, just that it's loading the right data to render what it wants to render. That's why we shouldn't mix up concepts and accidentally make the router aware of what it is it's loading. The route should kick off data fetching but then directly hand over to the actual route component. Anything else we do that's not directly intended to do a full route change should be completely opaque to the route renderer.

Not fully sure what you mean by the last part. Maybe you can clarify?

[Kingdutch]

So with the last part I was thinking about my topics page which has pagination and has the following prepare function:

~prepare=({ environment, at, direction, topicType }) => {
    let (first, after, last, before) = switch (at, direction) {
      | (Some(cursor), Some("backward")) => (None, None, Some(5), Some(Obj.magic(cursor)))
      | (Some(cursor), Some("forward"))
      | (Some(cursor), _) => (Some(5), Some(Obj.magic(cursor)), None, None)
      | (None, Some("backward")) => (None, None, Some(5), None)
      | (None, Some("forward"))
      | (None, _) => (Some(5), None, None, None)
    }

    let (topicType, filterByType) = switch(topicType) {
      | Some(topicType) => (Some(topicType), true)
      | None => (None, false)
    }

    let variables = TopicsPageQuery_graphql.Utils.makeVariables(~first=?first, ~after=?after, ~last=?last, ~before=?before, ~topicType=?topicType, ~filterByType, ())

    TopicsPageQuery_graphql.load(
      ~environment,
      ~variables,
      ~fetchPolicy=StoreOrNetwork,
      ()
    )
  },

Though I suppose the router only knows it needs to translate some path information into a query. It doesn't know that that is a paginatable fragment under the hood that can do its own reloading without navigation.

---

I'm starting to like your idea of a ShallowLink since it indicates that you're opting out of some of the preloading and having it restricted to what kind of navigations it can do ensures people don't accidentally break other navigations.

How would we deal with the distinction between preloading and actual navigation? Honestly the fact that I accidentally preloaded my next navigational page making the actual navigation instant was a big "wow" 🤩 Would love to make sure we can keep that functionality in peoples' pit of success :D

[Kingdutch]

For usage with useDynamicLink I do see challenges on the type front since if a component uses that but is used in different route hierarchies that may not have a specific set of query params defined then that function will break. So ReScript will probably kick-and-scream at us about this all the way.

Having said that one of the things my mind is wondering is whether we can't define query parameters separate from a route segment somehow (as an additional nested route for instance).

The example I gave above for topics is one of two places where I have pagination (and I expect to have that pattern all over my app). The related part of the routes.json file (for two usages) looks as follows:

      { "path": "/topics?topicType=string&direction=string&at=string", "name": "Topics" },
      { "path": "/events?direction=string&at=string", "name": "Events" }

I'd love to be able to say that I have some kind of pagination query parameters and I could have a Pagination component that would be smart enough to generate forward/backwards links.

With that in mind one thing that I should test for the above generalization is whether I could have two of those components on the same page (e.g. have two paginatable blocks side by side).

That last thought would actually work well if I could just have a "paginatable" query parameter segment because I could have both blocks in the URL but just have both of them namespaced to relate to the block I'm showing.

I hope you can still follow all that, I'm thinking out loud here 🙈

[zth]

Though I suppose the router only knows it needs to translate some path information into a query. It doesn't know that that is a paginatable fragment under the hood that can do its own reloading without navigation.

Correct. The router knows that it wants to start loading TopicsPageQuery. To do that, it's required to provide params. What the query then does with the params is none of the router's concern - it just kicks off that particular query and then forwards the query ref to the rendering component.

For usage with useDynamicLink I do see challenges on the type front since if a component uses that but is used in different route hierarchies that may not have a specific set of query params defined then that function will break.

I actually think this is fine, because all query params are always typed as options, and they're always decoded. So while it'll be perfectly possible to attempt to use a route's query params when you're not on that exact route, you won't get wrong/erronous values, because you'll always be forced to handle the fact that all query params might be missing.

I've had ideas before like providing a phantom type/variable to each route renderer, like a key, that you're forced to pass into that route's useQueryParams to access them. That would eliminate the possibility of using things in the wrong place. But, I've landed in instead viewing useQueryParams as a flexible "store selector" - it attempts to pull out something (query params) from the store (URL). If it's there, it's there. If not, it's not. Still required to handle all of those cases for the types to check out.

Having said that one of the things my mind is wondering is whether we can't define query parameters separate from a route segment somehow (as an additional nested route for instance).

I think this is interesting. Query params are already inherited from parent to child, but this is not about routes with that relation. If you don't care about having distinct query params for each of them, you can solve it today by simply having a custom query param like page=Pagination.QueryParam.t where Pagination.res could look something like this:

module QueryParam: {
  type t
  let serialize: t => string
  let parse: string => option<t>
} = {
  type t = { direction: direction, at: string }
  let serialize = t => `direction:${direction->Direction.toString}__at:${at}`
  let parse = str => ... // decode the string into `t` if things match
}

Your routes json would then be:

      { "path": "/topics?topicType=string&page=Pagination.t", "name": "Topics" },
      { "path": "/events?page=Pagination.t", "name": "Events" }

One possible alternative would of course be to have actual shareable route segments. Ignoring where exactly to define paginationParams in the example below, they could be used like:

      { "path": "/topics?topicType=string&...paginationParams", "name": "Topics" },
      { "path": "/events?...paginationParams", "name": "Events" }

But, I'm hesitant to add something like that for a number of reasons. In general, my stance can be summed up the same as why JSX spreads can be a foot gun - people end up using them for everything, accidentally bloating things, as well as straight out causing bugs like overwriting other params you're not supposed to overwrite, etc.

[Kingdutch]

I had missed (or forgotten) that query params are decodable! That's really cool actually. I think it makes sense to implement your proposal :D

[zth]

Awesome! I'll have a look at this soon.