Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Roadmap #13

Closed
taion opened this issue Apr 12, 2016 · 12 comments
Closed

Roadmap #13

taion opened this issue Apr 12, 2016 · 12 comments

Comments

@taion
Copy link
Owner

taion commented Apr 12, 2016

Per @gaearon's suggestion, I'm typing up the roadmap I have planned here, as a more technically oriented follow-up to my Medium post. I will probably eventually write this up on Medium.

My immediate main priorities development-wise are these:

More flexible path matching

This comes in two parts.

Richer path declaration syntax

The first is to switch from the home-built pattern matcher to path-to-regexp from Express. I have a very rough WIP on #12 – this is not ready to merge as-is. For earlier discussion, see remix-run/react-router#3105 and and tangentially remix-run/react-router#1923 (comment).

This path matcher is very similar to the existing one. It adds a major new feature in supporting regex constraints, so I can specify routes as e.g. ':widgetId(\\d+)' This will be very useful for constraining the types of parameters. In practice I expect this to be used like:

<Route path={`:widgetId(${UUID})`}>

with template strings, but only when needed – most route path specifications shouldn't change at all.

This also comes with two meaningful under-the-hood benefits – it removes a significant amount of rather fiddly code from our codebase, and per @KyleAMathews's benchmarks on remix-run/react-router#3215 (comment), is (somehow) an order of magnitude faster than even my optimized path matching logic released in [email protected]. It also makes things more consistent for people working full-stack in JavaScript, if they're using Express or one of the popular Koa routers.

This is slightly incompatible with the current path matching language, but only slightly – optional parameters and splats are handled a bit differently, and I want to move to Express's logic for repeated and anonymous route parameters, per remix-run/react-router#2422. I have an idea that I think works for deprecation strategy: #12 (comment). Essentially, we make the feature opt-in on a route-by-route basis (this is possible because of changes I made earlier in remix-run/react-router#2421), but in dev mode run both matchers in parallel, and issue deprecation warnings only on mismatches. This should avoid deprecation warnings for the vast majority of use cases, and give users a way to switch to the new syntax.

Conditional route matching

This is from @j's PR on remix-run/react-router#3098. Essentially, the problem is that if you have your routes set up like:

<Route path=":orgId" component={Org}>
  <Route path=":repoId" component={Repo} />
</Route>

You have no good way of rendering a "not found" view that doesn't include <Org>, if :repoId doesn't exist – there's no real API for a route to say "I do not match" in a dynamic way. This is related to the above, but orthogonal, as it involves dynamically (and perhaps asynchronously) checking for a route match and forwarding to an unmatched route if the dynamic check fails.

The best available option right now is to redirect in an async onEnter hook to e.g. /not-found, but I believe in general that this is not the correct approach.

You're well-entitled to ask at this point why I didn't just merge remix-run/react-router#3098, if I like it so much. The problem is that the current React Router route matching algorithm would lead to this feature leading to some very strange and unexpected behavior. I'll go into this later.

Scroll position management

This feature is just plain missing in the Router, and it's a pretty critical UX thing. I have the appropriate algorithms mostly coded up on scroll-behavior, but the library needs to be refactored to integrate to the router to properly handle async routes, rather than to integrate into history.

Why am I talking about this? Per discussion in remix-run/history#163, the pending v3 release of history is moving in a direction that makes it even more difficult to accomplish this in a reasonable way when using a hash history.

Furthermore, this should probably be integrated into the router – it's a core UX feature and shouldn't just be missing or opt-in. Proper scroll management should be present by default.

Route matching semantics

There's a few things wrong here. The biggest one is our handling of nested routes with absolute paths, per remix-run/react-router#3172.

To summarize, in order to support nested routes with absolute paths, we walk down all route branches with non-async children, even after the current path has failed to match, just in case there's a nested async route somewhere.

This is slow. It's not too noticeably right now for smaller route configs, since everything is synchronous, but would be absolutely awful with support for async route match conditions. As such, to implement async route match conditions requires removing this form of support for nested routes with absolute paths.

Per my updated docs in remix-run/react-router#3178, this pattern isn't even necessary any more – it's possible to decouple UI from URL with pathless routes, without any use of absolute paths in nested routes.

In principle it would be possible to build a mapping of absolute routes ahead of time, but it'd be extremely fiddly to preserve the existing route precedence semantics, so finally fixing this might actually require a breaking change. Instead I have a deprecation up on remix-run/react-router#3246, but note that it's not merged – even here.

Again, this is relevant not just for its own sake, but mostly so we can actually do dynamic route matching conditions without exhaustively evaluation all preceding ones.

Link active semantics

As a follow-up to the above, and to remix-run/react-router#3158, which removes some silly handling of extraneous slashes in the router (for whatever reason, we make the pattern /foo/bar match the path /foo////////bar//////), we can also dramatically simplify link active state semantics, per remix-run/react-router#3231.

Instead of running a match right now, we can just do a prefix match with some careful handling of trailing slashes. This would allow us to completely bypass issues like remix-run/react-router#3277, which are not wrong per se, but actually just confusing complications of the current "active" semantics.

It would also be a lot faster, which would address the unresolved (but closed) remix-run/react-router#1873.

Async route handling and code splitting

This is from remix-run/react-router#3232, which I believe was closed without proper consideration.

I'm not going to go into as much detail here, as I sketch out the problem in some concrete detail in the linked issue, but essentially the current route-based code splitting API in React Router is not ideal, in that it will lead to suboptimal results if used in the most straightforward way.

The problem with an API like getChildRoutes is that it doesn't look at which of the child routes might potentially match, so it always leads to overfetching through loading all of the child routes whenever any child route might be active. Furthermore, when used in conjunction with an async getComponent callback (which will mitigate the overfetching), this leads to async route loads requiring two server round-trips.

Per remix-run/react-router#3259, most applications shouldn't be using getChildRoutes or getIndexRoute at all and should just be using getComponent or getComponents.

For applications that do need route-based code splitting for whatever reason, the split point should be after the match, with something like my getRouteConfig proposal, such that we load only a chunk with only the relevant route, rather than a chunk containing all child routes as happens right now in the huge-apps example with getChildRoutes.

Other features

There are a few other miscellaneous features I haven't fully thought through, or don't need much of a writeup, but I'll list them for completeness:

Release schedule and project management

I don't know how much there is to say here. I'd like to cut feature releases on a regular basis – perhaps monthly, but never with more than one or two deprecations at a time. I'd like to cut bugfix releases as soon as they're merged – actually, I'd like to cut all releases as soon as they're merged, because I think long-running divergences between master and the latest release are unhealthy, except in the case of breaking changes.

I use this library, and I don't particularly enjoy migrating my code to use new APIs. API stability is an absolute priority. #12 once ready will almost certainly include more code for handling backward compatibility than for implementing the feature.

Also, I'm thinking that moving toward a policy of "no questions on the issue tracker" has been very counterproductive. Given such an issue, the right policy ought to be to answer no questions on the issue tracker, to be consistent and to avoid "rewarding" (I use that term sarcastically here) people who break the issue filing guidelines.

However, basic human decency makes that policy almost impossible to follow without the conversations growing extremely contentious. As such, it'd probably be best to address simple questions on the issue tracker as possible (e.g. when they can be resolved with a simple pointer to the relevant section of the docs), but redirect more complicated ones to Stack Overflow or Reactiflux. This implicit policy has worked quite well for us on React-Bootstrap, and I think it's relatively scalable, given that React Router is a small project in terms of API size, unlike, say, Babel, for which this approach is less likely to succeed.

ETA: One of the explicit goals here is to minimize API changes whenever possible, except when absolutely necessary. For example, I want to minimize changes like moving from context.history to context.router from react-router@1 to react-router@2 that don't provide serious improvements to users.

@michaelBenin
Copy link

This lib might help to serve inspiration with what's first: https://visionmedia.github.io/page.js/

@taion
Copy link
Owner Author

taion commented Apr 12, 2016

@michaelBenin Thanks. I guess I forgot to mention it above (I'll ETA), but one of the explicit goals here is in fact to minimize API changes, so major divergences from the React Router v2 API are probably out of the picture.

@taion
Copy link
Owner Author

taion commented Apr 12, 2016

Another addition: I'm also really fond of the React-Bootstrap contribution policy, which requires all non-trivial changes go through PRs that are signed off by at least two contributors to the project. I'd like to move to that pattern ASAP, as soon as I actually have other collaborators on this project.

@taion
Copy link
Owner Author

taion commented Apr 12, 2016

One more note – I'm considering moving some commonly used extensions that are agnostic to other libraries into the core of this project in the near term.

The two I'm looking at are:

  • use-named-routes, which adds first-class named route support as a history enhancer, that makes all router functionality work with route names.
  • <LinkContainer> and <IndexLinkContainer> from React-Router-Bootstrap, which allow adding router link functionality to custom React components, by injecting down href, an active prop (configurable by something like activePropName), and an onClick handler. Despite the library name, these components have almost nothing to do with React-Bootstrap.

I welcome any additional feedback here.

@Furizaa
Copy link

Furizaa commented Apr 12, 2016

Is it your general idea to keep easily extractable functionality within the core of the project? I'm asking because IMHO it would be a nice approach to make a common route-matching library that can be used if your plan is to rework that part anyway.

If you want to have that in the core then that is totally fine with me. I just want to get a general feel for where this project is heading.

@taion
Copy link
Owner Author

taion commented Apr 12, 2016

@Furizaa Can you clarify what you mean by "easily extractable functionality"?

@Furizaa
Copy link

Furizaa commented Apr 12, 2016

Ok, that was a poor choice of words on my side. Cut the "easy" 😃 . But I think the route matching is a prime example here. If I remember correctly there was a discussion on the React-Router project about moving the matching out of the core like it was done with the history. Is this something you would embrace or do you like to keep things in the core?

If this all makes no sense than that's also fine. I Occasionally have to dig in the source of the router but I'm far to detached to make sense of it all.

@taion
Copy link
Owner Author

taion commented Apr 12, 2016

@Furizaa

It depends. You might be talking about my remix-run/react-router#2286, or else a related issue.

We were thinking of making route matching be customizable to allow something like the regex-based path matching I've mentioned above.

At this point, I think using path-to-regexp plus the condition check would offer an acceptable solution in almost all use cases, so I don't think it'd immediately make sense to offer further customizability for route pattern matching.

However, one consequence of the above change is that the details of pattern matching would be out of core! They'd live upstream in the path-to-regexp dependency.

@taion
Copy link
Owner Author

taion commented Apr 13, 2016

Crossed out one of the minor quality-of-life things that I just implemented.

@freddyrangel
Copy link

One minor detail that might be worth mentioning: perhaps we could use a different Discord chat on the README. The current one goes to React Router.

@taion
Copy link
Owner Author

taion commented Apr 13, 2016

That might become necessary at some point. I'd rather not clutter up Reactiflux more than necessary, though.

@taion
Copy link
Owner Author

taion commented Apr 13, 2016

Thanks for everyone's feedback. I'm folding this project back into React Router proper. Please stay tuned for a proper announcement. I'm deeply sorry for all the inconvenience this has caused, and I thank you all for your patience.

@taion taion closed this as completed Apr 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants