Support for React Router v4

[Xesme]

Is your feature request related to a problem? Please describe.
When adding us-forms-system to an existing project, it is very frustrating there you cannot use React Router v4.

Describe the solution you'd like
I would be helpful if support for React Router v4 or there was documentation on how to support React Router v4 in an existing project.

Describe alternatives you've considered
a) Downgrade React Router v4 to React Router v3 and reverse engineer the migration guide for an existing project.
b) Fork and add support for React Router v4 and file a PR.
c) Buy an alpaca farm in South America and live a simpler life.

Additional context

createRoute() creates a route object, but React Router v4 does not support route objects.

[dmethvin-gov]

That alpaca farm is starting to look good to me too. Can you describe how you're using us-forms-system in your existing app? I think our existing users are pretty much building standalone apps that consist only of the forms, so we probably don't integrate well yet.

[Xesme]

We have added the us-form-system to an existing application. It replaces the form component within the app. We were using react-router v4 in the project, but have downgraded at this time to use v3. Looking forward to having support for react-router v4 in the future.

[dmethvin-gov]

Is there an open-source repo we could look at?

In general I think of routing as an app-level thing and not a library-level thing, so perhaps USFS should not include a router but instead integrate with whatever router the app chose. I am not sure how that would work, probably some function that you would call to return a list of pages and routes.

[jcmeloni-usds]

@dmethvin-gov What are your thoughts on a little discovery spike on this in the near future?

[dmethvin-gov]

@jcmeloni-usds The hackathon may give us good feedback on use cases, and then we need to decide whether they are in scope or not. If we separated out the navigation from the form management and treated them as two separate components (which was also wanted in #169) I think this problem would go away.

[jcmeloni-usds]

@dmethvin-gov That makes sense

[annekainicUSDS]

It looks like there are other teams that are experiencing the same issue with not being able to use react-router v4. I'm going to do some investigation on what the upgrade would entail, and find out what kind of impact this will have on Vets.gov and if they could also upgrade their apps to react-router v4.

[jcmeloni-usds]

I glanced at https://codeburst.io/react-router-v4-unofficial-migration-guide-5a370b8905a and went "ugh -- that looks like a solid week to migrate all the vets.gov apps and then get through testing." YMMV.

I know we talked about making RR a configuration choice, but it seems like that would mean more refactoring on this side to handle that, then the other way around. Also, I could be wrong.

What do you think about freezing an publishing a version as-is so that vets.gov can keep their dependency on prev version, and just diving in to make the lib useable w/ RR4. I would consider it a top priority -- unless @ju-liem has a specfically different master plan.

[annekainicUSDS]

@jcmeloni-usds I was thinking the same thing, that it seems like an easier path to upgrade react-router for now as opposed to breaking out the routing as a separate library. This sounds like a request from multiple teams, and I think it's important to start making this library usable by new teams.

If our only hold-up is the fact that Vets.gov depends on react-router 3, they could definitely just use the last published version of us-forms-system before the upgrade until they are ready to upgrade their apps. From an initial conversation I had with their team, it sounds like their roadmap of work is pretty full right now, so I'm not sure how quickly they could take on the upgrade work, but I'll keep asking and see what their timeline looks like.

I started doing some digging into what exactly this work will entail on our side, so I'll start documenting that in this ticket!

[dmethvin-gov]

Since React Router 4 is the latest and greatest, it definitely seems like we should be using it. By the time our project gets wide adoption (hoping that it does) version 3 will be pretty stale. So as far as that part goes I'm 👍.

As far as keeping the router in the library vs the starter app I think it's unusual for a library (versus an app) to make those kind of decisions. Some router-related decisions can affect several other things that may be out of the control of the front-end developer who wants to use the library. That will limit adoption.

For example, we use push-state at the moment rather than hashes. Our URLs don't contain form state that would allow someone to start in the middle of a form and the URLs don't mean anything to the server that would allow it to do that either--they don't represent server resources. (See this article for some background.) If you can't configure the server to redirect those "fake" server URLs back to the start of the form, the user will get 404 errors.

[annekainicUSDS]

@dmethvin-gov agreed that we should continue to explore separating the routing from the library, and that might be the goal we want to work towards eventually. However, there are a few reasons I think it might make more sense to do the upgrade to react-router v4 first:

1. We know that upgrading to react-router 4 is a current feature request of our library, and that it's currently blocking adoption of the library by some users. Conversely, we don't yet know if users want to be able to completely control how routing works for their apps, for things you mentioned like form state in hashes. We suspect it may eventually be an issue for some hypothetical future user, but we don't yet have any data (i.e., real users asking for this feature) to tell us that this is a worthwhile feature to work on.
2. Upgrading the router within the library first is an incremental step towards refactoring how we handle routing in general. As you said, react-router 4 is the latest and greatest, and most users are probably going to use it. If we eventually pull the routing out of us-forms-system and into us-forms-system-starter-app, we will in all likelihood be using react-router 4 in the starter app, so if we make that upgrade now we've made an incremental steps towards that eventual goal.

In general, when deciding on what engineering work to take on, I would like to prioritize work that we know is a current blocking issue (based on direct feedback from users) and to break our work into incremental stages. The kinds of refactoring work we're considering is pretty huge, and if we can break it down into chunks that build towards an end goal, I think we'll be much better off.

[dmethvin-gov]

I probably am not explaining myself very well and making it sound a lot more complicated than it is, so I did a couple of quick changes to show what I mean. This branch of USFS removes the dependency on React Router 3 and this branch of the starter app moves the router-version-specific route creation to the starter app. At this point the starter app is still running RR3 but in some ways that's better. It should be possible for vets.gov to update their version with very few changes and stay on RR3 if they want. However, now that we don't wire in the RR3 dependency the starter app could be upgraded to RR4. At least that would be the theory.

[annekainicUSDS]

These changes aren't really in line with what I understood the purpose of the starter app to be, which is just to set up the initial files you need in order to focus on just building the form config, not to produce essential pieces of functionality that the library depends upon in order to work. Creating the routes by reading the form config, to me, is an essential piece of functionality. Without that, the form config isn't really useful because it won't know how to render anything that's in there.

By pulling this into the starter-app, users of the us-forms-system that aren't using the starter app, like Vets.gov, will have to reproduce this functionality in their own codebase, which I don't think is ideal. I think we need to think through in more detail what the bounds of us-forms-system is, and what are the most essential pieces needed in order for it to still be useful and be capable of building a form, before we start pulling things out.

[dmethvin-gov]

By pulling this into the starter-app, users of the us-forms-system that aren't using the starter app, like Vets.gov, will have to reproduce this functionality in their own codebase

I was just reading the formData saving functionality mentioned in the email and it looks like they already do reproduce this since the default functionality doesn't do what they want. That also leads to a strange situation in the components where we expose both the withRouter() versions and the raw components. I didn't know why that was done until I today when I read their code and realized that vets.gov can't use the one we create in the library. Basically, vets.gov is already doing what I proposed above (but in a less hacky way that what I did in the PR since they call connect in their code).

[annekainicUSDS]

From the conversation @dmethvin-gov and I just had on the phone, I think we're going to start by upgrading to react-router version 4 in us-forms-system and the starter app, and then we'll do discovery on pulling out routing functionality after that.

[afeld]

I think of routing as an app-level thing and not a library-level thing, so perhaps USFS should not include a router

cc #169

[peterpme]

👋 I'd like to help in separating the router from the form-builder whenever the time comes. I've built something similar for draftbit.com (our drag / drop mobile app builder) and have extensive experience with both react-router 4 & the original json schema library.

Thanks!

[annekainicUSDS]

There are potentially some issues with react-router 4 and the newer features in recent versions of React 16 (specifically with the context api), but I don't think this is a problem in the first versions of React 16. There is also another router alternative, reach router, that could be an alternative if react-router 4 is ever causing us problems.

[annekainicUSDS]

@peterpme thanks for your contribution! We will definitely reach back out to you when we get to the point of separating the router from the core form library. Thanks for offering your help!

[jcmeloni-usds]

Just an update for all interested parties on this. First, thanks for your patience!

As the team is finishing up Phase 1 of the project (end of Sept) and moving into Phase 2 (literally the next day), we plan to cut a stable 1.x release of the library as-is with the routing as it is, and almost immediately thereafter publish a 2.0.0-alpha.1 version that is the library as-is but with React Router 4. A subsequent 2.0.0-alpha.2 version will take another step forward with RR4 in place and the lib upgraded to current version of React, and we'll march forward from there while keeping the 1.x stable for existing users.

We'll talk about this on the next contributor call -- hope everyone can join!

[easherma]

@annekainicUSDS

We're contemplating running with updating this repo to use react router 4 to suit our purposes at City of Austin.
I noticed this PR:
https://github.com/usds/us-forms-system/tree/upgrade-to-react-router-4

But at first glance I'm not sure how far along it is. Did ya'll have a more detailed process/todo-list ready to go on this issue that we could take and run with?

[dmethvin-gov]

Anne and I looked at this a while back and the differences between RR3 vs RR4 are very large. That made the upgrade messy, particularly because we have routing spread between the library here and the starter-app. If there's something you can get out of the partial changes in the branch then go for it. It's probably easier to start fresh though so you know where things stand. As mentioned above I think it would be easiest to remove the RR dependency from the library here, put it into the starter app (or your app), and have the library call a prop that sets the route. During setup, the library can pass back an object to the app and you create routes from that to fit into the app.