React-router: Rendering component wrapped in `withRouter` throws when no Router is present

[dgcoffman]

Version

4.0.0, 4.0.0-beta.8

Test Case

import React from 'react' // 15.4.2
import renderer from 'react-test-renderer' // 15.4.2
import { withRouter } from 'react-router'

const Component = () => <div />
const WrappedComponent = withRouter(Component)

it('will render', () => expect(renderer.create(<Component />)).toBeDefined())
it('will fail', () => renderer.create(<WrappedComponent />))

Steps to reproduce

Run above tests in Jest, or any other test runner.

Expected Behavior

Rendering WrappedComponent with react-test-renderer does not fail.

Actual Behavior

  ● will fail

    TypeError: Cannot read property 'route' of undefined

      at Route.computeMatch (node_modules/react-router/Route.js:66:22)
      at new Route (node_modules/react-router/Route.js:43:20)
      at node_modules/react-test-renderer/lib/ReactCompositeComponent.js:295:18
      at measureLifeCyclePerf (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:75:12)
      at ReactCompositeComponentWrapper._constructComponentWithoutOwner (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:294:16)
      at ReactCompositeComponentWrapper._constructComponent (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:280:21)
      at ReactCompositeComponentWrapper.mountComponent (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:188:21)
      at Object.mountComponent (node_modules/react-test-renderer/lib/ReactReconciler.js:46:35)
      at ReactCompositeComponentWrapper.performInitialMount (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:371:34)
      at ReactCompositeComponentWrapper.mountComponent (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:258:21)
      at Object.mountComponent (node_modules/react-test-renderer/lib/ReactReconciler.js:46:35)
      at ReactCompositeComponentWrapper.performInitialMount (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:371:34)
      at ReactCompositeComponentWrapper.mountComponent (node_modules/react-test-renderer/lib/ReactCompositeComponent.js:258:21)
      at Object.mountComponent (node_modules/react-test-renderer/lib/ReactReconciler.js:46:35)
      at mountComponentIntoNode (node_modules/react-test-renderer/lib/ReactTestMount.js:55:31)
      at ReactTestReconcileTransaction.perform (node_modules/react-test-renderer/lib/Transaction.js:140:20)
      at batchedMountComponentIntoNode (node_modules/react-test-renderer/lib/ReactTestMount.js:69:27)
      at ReactDefaultBatchingStrategyTransaction.perform (node_modules/react-test-renderer/lib/Transaction.js:140:20)
      at Object.batchedUpdates (node_modules/react-test-renderer/lib/ReactDefaultBatchingStrategy.js:62:26)
      at Object.batchedUpdates (node_modules/react-test-renderer/lib/ReactUpdates.js:97:27)
      at Object.render [as create] (node_modules/react-test-renderer/lib/ReactTestMount.js:125:18)
      at Object.<anonymous> (src/components/Button/Button-snapshot.js:9:118)
      at process._tickCallback (internal/process/next_tick.js:109:7)

Route.computeMatch (node_modules/react-router/Route.js:66:22) is

  Route.prototype.computeMatch = function computeMatch(_ref, _ref2) {
    var computedMatch = _ref.computedMatch,
        location = _ref.location,
        path = _ref.path,
        strict = _ref.strict,
        exact = _ref.exact;
    var route = _ref2.route; // _ref2 is undefined

Passing { withRef: true } to withRouter has no effect.

This makes it hard to unit test any component that renders as one of its children a component wrapped in withRouter.

Can be worked-around by inserting a Router.

import { MemoryRouter as Router, withRouter } from 'react-router-dom' // 4.0.0

it('will pass', () => {
  expect(
    renderer.create(<Router><WrappedComponent /></Router>)
    .toJSON()
  ).toEqual({children: null, props: {}, type: 'div'})
})

Thanks!

possibly related bugfix here https://github.com/ReactTraining/react-router/issues/4292

[pshrmn]

The testing guide covers the necessity of rendering inside of a <Router>.

I'm not sure if react-test-renderer allows you to pass in a context object, but if it does you can use react-router-test-context to simulate the context a router would provide. While I wrote it, I actually believe that it is usually better to just use a <MemoryRouter>. From my limited experience testing it, shallow rendering does not work well with <Route>s.

Also, I'm not positive, but is your reference to withRef just related to computeMatch having _ref and _ref2 as arguments? That is just a babel thing. I'm not actually sure how ref works with withRouter because I haven't had a need to use them together. I know that some HOCs have to do some hoisting, but I haven't seen anyone bringing up having issues with that yet.

[dgcoffman]

Also, I'm not positive, but is your reference to withRef just related to computeMatch having _ref and _ref2 as arguments?

No, it's related to when that existed:
https://github.com/ReactTraining/react-router/pull/3735
https://github.com/ReactTraining/react-router/pull/3740

Anyway, it'd be better if a component using withRouter didn't throw when mounted outside a Router for a variety of reasons -- which is why y'all merged a bugfix for this same issue in December of 2016: https://github.com/ReactTraining/react-router/issues/4292

Hope you reconsider closing this.

[pshrmn]

I'll re-open for the time being for the ref issue, but I can say that I'm about 99% sure adding functionality to components just for testing purposes isn't going to happen.

[dgcoffman]

There are non-testing applications. Imagine a component that is used in two component trees -- one having a Router and another not.

This actually happened in our codebase just moments ago

[Anahkiasen]

but I can say that I'm about 99% sure adding functionality to components just for testing purposes isn't going to happen.

I'm not sure I get your reasoning, I like RR4 so far but if it that means I can't test my components I'm going to have to revert to using RR3. I have a component A that renders a component withRouter(B).

Currently I can't test A because if I don't wrap it the test crashes, and if I wrap it in per example a MemoryRouter then I can't use most of enzyme's methods (setState etc.) as they can only be called on the root component, which is now the router. Note that B needs the router in an inconsequential way (it needs it for a logout action, not for rendering).

Maybe there's a better way I haven't thought of though, I checked a bit on both enzyme and RR's repositories and didn't find much but I haven't worked with RR4 for long so :/

[mjackson]

It makes sense that if you wrap a component in something called withRouter that it wouldn't work correctly if you don't render it in the context of a <Router>. If you didn't, what props would you expect to receive?

[dgcoffman]

If you didn't, what props would you expect to receive?

undefined or whatever I supply manually

It would be most convenient if withRouter with no <Router> was a no-op, like this PR made it https://github.com/ReactTraining/react-router/pull/4295 (unfortunately since regressed).

Throwing seems like the least desirable thing. I could see a warning maybe.

For any readers: I now export the unwrapped component by name, import that in my test files, and supply my own route prop mock objects.

[mjackson]

Maybe I can phrase it differently: if you're not planning on rendering a component inside a <Router>, why are you wrapping it in withRouter?

// This makes sense to me.
<Router>
  {withRouter(MyComponent)}
</Router>

// This doesn't.
<div>
  {withRouter(MyComponent)}
</div>

What's preventing you from just not using withRouter when you unit test your component?

[dgcoffman]

if you're not planning on rendering a component inside a , why are you wrapping it in withRouter

Because that's how my application code uses it.

What's preventing you from just not using withRouter when you unit test your component?

For shallow rendering a single component: Nothing, that's what I'm doing now to work around this.

It requires me to export the unwrapped component and import that in my test, which is a mild inconvenience.

Imagine the case where you are, in a unit test, deep rendering your component under test, and it renders somewhere in its descendent tree a component that is wrapped in withRouter. How then should we just not use withRouter in that child?

Module-level mock the child to inject an unwrapped component? Gets messy quickly.

We ended up inserting a <Router> in our tests for those cases. This isn't ideal because it breaks some enzyme functionality that can only operate on the root rendered element.

Crux of the issue is that I don't expect withRouter to throw in this case, and think it would be nicer if it behaved as it did after https://github.com/ReactTraining/react-router/pull/4295 was applied to fix this issue once before.

[timdorr]

Perhaps a noop with a warning message would be best?

[mjackson]

We ended up inserting a <Router> in our tests for those cases.

That's the way all of our tests work here; they all render a <Router> at the top of the component hierarchy.

I get that this is really inconvenient for you, but I'm just not sure the best way to fix it. What does redux do if you render one of your connected components without a <Provider> up top? Does it just give you no props and a warning?

[timdorr]

We fire an invariant, but we have a special case of not requiring a context provider, as we also support the store prop. It's a little different from our use case here.

But I think #4939 will cover this if we can get it merged.

[dgcoffman]

I'm just not sure the best way to fix it

My vote is for

if (!router) {
  return <WrappedComponent {...this.props} />
}

[mjackson]

Well, #4939 is now merged which is going to yell at people if they try to use any of <Route>, <Switch>, <Link>, or withRouter outside the context of a <Router>. I personally think this makes a lot of sense: don't use our API if you're not going to render a <Router> up top. At least that should help people to know what they're doing wrong.

I'm open to ideas, but I don't like the thought of making withRouter (or any of our API) a no-op when there's no router on context. That just opens up a whole category of issues that I don't believe we should be responsible for addressing.

[akihikodaki]

What's the conclusion for this case mentioned the above:

There are non-testing applications. Imagine a component that is used in two component trees -- one having a Router and another not.

This actually happened in our codebase just moments ago

But all the later discussions are about testing. We are actually experiencing the issue with non-testing application; we have an complete React application with routing and React components which are just a part of pages generated by Rails.
The components as parts of Rails pages should not manipulate history. Instead, it should navigate to the page by requesting the destination page.
See https://github.com/tootsuite/mastodon for our case. In the case,

• The landing page is generated by Rails, but contains a preview built with React (the part titled with "a look inside" at https://mastodon.social/about.) Actions using history are prohibited or delegated to actual navigation.
• The page after signing in is built with React and uses react-router and it handles all routing.

[SteveGBanton]

@dgcoffman

We ended up inserting a in our tests for those cases. This isn't ideal because it breaks some enzyme functionality that can only operate on the root rendered element.

If I'm understanding your problem correctly, you need to operate on the root component with enzyme and you'd rather not wrap in a router. We've been mocking out the context on the mount and shallow methods in our tests... Maybe it'll work for you - for sample code you can see here, hope that helps if you're still having the issue :)

[EdGraVill]

Use react-router-dom mock like this:

import React from 'react' // 15.4.2
import renderer from 'react-test-renderer' // 15.4.2
import { withRouter } from 'react-router'

jest.mock('react-router-dom'); // <- Use this line

const Component = () => <div />
const WrappedComponent = withRouter(Component)

it('will render', () => expect(renderer.create(<Component />)).toBeDefined())
it('will fail', () => renderer.create(<WrappedComponent />))