Add redirect, props, forcedProps to react-router-config route keys and inject renderChild function prop

[jharris4]

TLDR

Adds redirect, props, forcedProps as keys to route:

const routes = [
  {
    component: Root,
    routes: [
      {
        path: "/",
        exact: true,
        component: Home
      },
      {
        path: "/other:id",
        redirect: "/child:id"
      },
      {
        path: "/child/:id",
        component: Child,
        props: {
          className: "child-css-class"
        },
        routes: [
          {
            path: "/child/:id/grand-child",
            component: GrandChild
          }
        ]
      }
    ]
  }
];

And passes a renderChild = props => renderRoutes(route, { extraProps: props }) function to all route components:

const Root = ({ renderChild }) => (
  <div>
    <h1>Root</h1>
    {/* child routes won't render without this */}
    {renderChild()}
  </div>
);

const Home = ({ route, renderChild }) => (
  <div>
    <h2>Home</h2>
  </div>
);

const Child = ({ renderChild }) => (
  <div>
    <h2>Child</h2>
    {/* child routes won't render without this */}
    {renderChild({ someProp: "these extra props are optional" })}
  </div>
);

const GrandChild = ({ someProp }) => (
  <div>
    <h3>Grand Child</h3>
    <div>{someProp}</div>
  </div>
);

Docs and tests updated accordingly

[jharris4]

I'm having a little trouble with the build. All the tests are passing just fine locally...

Any pointers on how to fix the build would be greatly appreciated :-)

[timdorr]

Don't touch the package.json file. You've updated a bunch of dependencies, so it's no longer able to build.

Also, I wouldn't worry about cleaning up the readme in this PR. Just focus on the code and tests for now.

[jharris4]

@timdorr I didn't intentionally update a bunch of dependencies. I only changed the dependency on react-router to "react-router": "^4.3.0-rc.3"

This PR makes use of the new generatePath so it won't work without the most recent react-router version...

In any case, I've reverted my commit of the change to package-lock.json, but not the commit to package.json.

Any ideas what I'm missing to make the tests pass with npm in the monorepo? They work beautifully for me when I use yarn... :-)

[pshrmn]

RR uses lerna to link the different packages, so while the package.json may say the dependency is on a previous release, it will actually be using the local version.

[jharris4]

I'm quite familiar with lerna :-)

Here is what I tried before making the PR to get the tests to run from the root of the mono repo:

git clone https://github.com/ReactTraining/react-router.git
cd react-router
npm install
npm run build
npm test

So first off there are tests failing when I do that. (Filed an issue #6174)

Then I tried disabling the failing tests applying the changes from this PR and re-ran the tests from the mono repo root:

modules/index.js → umd/react-router-config.js...
[!] Error: 'default' is not exported by ../react-router/Redirect.js
https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module
modules/renderRoutes.js (4:7)
2: import Switch from "react-router/Switch";
3: import Route from "react-router/Route";
4: import Redirect from "react-router/Redirect";
          ^
5: import generatePath from "react-router/generatePath";

This is where things get funky, weird errors starting happening where imports from packages/react-router are not resolved properly.

I've been trying to pin down what the issue is but that's a bit difficult to do when the tests on master we not passing to begin with. :-(

[pshrmn]

I was just digging in to how Lerna checks for local dependencies and you were right to update the package version. Lerna uses semver.satisfies() to check if it can use a local package and the -rc versioning doesn't satisfy a non -rc dependency.

[jharris4]

I had a little spare time this morning so I tinkered with the react-router-config build scripts to get the build and tests working from the mono repo root.

I've updated this PR with those changes, but I had to mimic the CI and disable the website build until somebody comes up with a way to make it run in a separate build phase after the other packages.

I've had luck managing this in mono repos by splitting all the package.json script names across the mono repo packages into categories. build (runs babel) bundle (runs rollup) deploy (runs webpack).

That makes it a lot easier to just do this from the mono repo root and get all the things in the right order:

lerna run build
lerna run bundle
lerna run deploy
lerna run test

It might be helpful to consider a similar strategy for this repo... A quick version of this would be to update the website package.json script name to be something other than build...

Also note that the rollup-config.js and babel-preset.js of react-router-dom needed tweaks for me to get the build passing from the mono-repo root. The same changes I pushed to those files for react-router-config are needed for the whole build to pass.

I only didn't commit those changes because it's outside the scope of this PR...

[jharris4]

I merged the latest from master (4.3.1) and the tests are passing now.

I also added the latest changes to adds an addParamProps option as a convenience to inject match.params.props directly as props when desired.

Please let me know if there's anything I can do to help get this merged.

[jharris4]

@timdorr @mjackson Is there any likelihood that this will eventually get merged?

In the interim, we've been using the forked package react-router-routes.

I won't bother trying to fix the merge conflicts here unless it's likely to get merged... ;-)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jharris4]

Well, I gotta say it’s kinda frustrating that these great & useful additions to the package have stagnated for a lack of attention.... 😞

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[timdorr]

I marked this so it won't be touched by stalebot again. Can you rebase against the latest master?

[jharris4]

@timdorr I’m doing some holiday traveling at the moment but will take a look at rebasing whenever I get the chance, probably early next week.

[jharris4]

I just spent a couple of hours trying to rebase and it got really messy and confusing due to all the other changes that have happened in the 1.5 years since I last worked on this PR.

I did however manage to get all the tests passing locally against master with the changes & added tests from this PR.

If somebody can tell me that the features I added are likely to be merged then I'll find some more time to work on this.