Rendering component wrapped in `withRouter` throws when no Router is present #4795

dgcoffman · 2017-03-21T09:16:45Z

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 #4292

The text was updated successfully, but these errors were encountered:

pshrmn · 2017-03-22T16:14:34Z

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 · 2017-03-22T22:43:30Z

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:
#3735
#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: #4292

Hope you reconsider closing this.

pshrmn · 2017-03-22T22:51:50Z

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 · 2017-03-22T22:52:34Z

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 · 2017-06-27T09:13:30Z

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 · 2017-07-14T05:49:58Z

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 · 2017-07-14T05:58:34Z

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 #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 · 2017-07-14T06:15:43Z

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 · 2017-07-14T06:20:31Z

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 #4295 was applied to fix this issue once before.

timdorr · 2017-07-14T15:36:09Z

Perhaps a noop with a warning message would be best?

mjackson · 2017-07-14T17:44:17Z

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 · 2017-07-14T19:05:55Z

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 · 2017-07-14T20:40:01Z

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

My vote is for

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

mjackson · 2017-07-14T22:56:25Z

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 · 2017-10-03T12:33:11Z

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:/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 · 2018-05-20T19:19:23Z

@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 · 2018-06-28T16:42:14Z

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 />))

pshrmn closed this as completed Mar 22, 2017

pshrmn reopened this Mar 22, 2017

agundermann mentioned this issue Mar 25, 2017

Improve withRouter #4838

Merged

mjackson closed this as completed Jul 14, 2017

jorgeitenjr mentioned this issue Sep 14, 2017

Invariant Violation: You should not use <Route> or withRouter() outside a <Router> enzymejs/enzyme#1112

Closed

remix-run deleted a comment from webmobiles Oct 18, 2017

lock bot locked as resolved and limited conversation to collaborators Aug 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rendering component wrapped in `withRouter` throws when no Router is present #4795

Rendering component wrapped in `withRouter` throws when no Router is present #4795

dgcoffman commented Mar 21, 2017 •

edited

Loading

pshrmn commented Mar 22, 2017

dgcoffman commented Mar 22, 2017 •

edited

Loading

pshrmn commented Mar 22, 2017

dgcoffman commented Mar 22, 2017 •

edited

Loading

Anahkiasen commented Jun 27, 2017 •

edited

Loading

mjackson commented Jul 14, 2017

dgcoffman commented Jul 14, 2017 •

edited

Loading

mjackson commented Jul 14, 2017

dgcoffman commented Jul 14, 2017 •

edited

Loading

timdorr commented Jul 14, 2017

mjackson commented Jul 14, 2017 •

edited

Loading

timdorr commented Jul 14, 2017

dgcoffman commented Jul 14, 2017

mjackson commented Jul 14, 2017

akihikodaki commented Oct 3, 2017

SteveGBanton commented May 20, 2018 •

edited

Loading

EdGraVill commented Jun 28, 2018

Rendering component wrapped in withRouter throws when no Router is present #4795

Rendering component wrapped in withRouter throws when no Router is present #4795

Comments

dgcoffman commented Mar 21, 2017 • edited Loading

Version

Test Case

Steps to reproduce

Expected Behavior

Actual Behavior

pshrmn commented Mar 22, 2017

dgcoffman commented Mar 22, 2017 • edited Loading

pshrmn commented Mar 22, 2017

dgcoffman commented Mar 22, 2017 • edited Loading

Anahkiasen commented Jun 27, 2017 • edited Loading

mjackson commented Jul 14, 2017

dgcoffman commented Jul 14, 2017 • edited Loading

mjackson commented Jul 14, 2017

dgcoffman commented Jul 14, 2017 • edited Loading

timdorr commented Jul 14, 2017

mjackson commented Jul 14, 2017 • edited Loading

timdorr commented Jul 14, 2017

dgcoffman commented Jul 14, 2017

mjackson commented Jul 14, 2017

akihikodaki commented Oct 3, 2017

SteveGBanton commented May 20, 2018 • edited Loading

EdGraVill commented Jun 28, 2018

Rendering component wrapped in `withRouter` throws when no Router is present #4795

Rendering component wrapped in `withRouter` throws when no Router is present #4795

dgcoffman commented Mar 21, 2017 •

edited

Loading

dgcoffman commented Mar 22, 2017 •

edited

Loading

dgcoffman commented Mar 22, 2017 •

edited

Loading

Anahkiasen commented Jun 27, 2017 •

edited

Loading

dgcoffman commented Jul 14, 2017 •

edited

Loading

dgcoffman commented Jul 14, 2017 •

edited

Loading

mjackson commented Jul 14, 2017 •

edited

Loading

SteveGBanton commented May 20, 2018 •

edited

Loading