Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HOC withRouter causes TestUtils renderIntoDocument to 'fail' silently #3652

Closed
jrobison153 opened this issue Jul 20, 2016 · 12 comments
Closed

Comments

@jrobison153
Copy link

Description

wrapping a React component with HOC withRouther causes React TestUtils renderIntoDocument to fail silently. I'm using a mocha/chai/sinon test stack, I will try to carve out some time to pull together a gist if needed.

Version

react-router 2.5.2
react 0.14.1

Steps to reproduce

  1. export your React component wrapped in the recommended withRouter HOC
  2. In your unit test code render the component using TestUtils.renderIntoDocument

YourComponent.jsx

var React = require('react');
import { withRouter } from 'react-router'

class YourComponent extends React.Component {

    render() {

        return <div>

        </div>;
    }
}

module.exports = withRouter(Info); //remove withRouter and everything is fine.

YourComponentTests.js

var YourComponent = require('../YourComponent.jsx');

....

 beforeEach(function () {

            // note that this does not fail when component is wrapped withRouter it just doesn't
           // return a rendered react component
            yourComponent = TestUtils.renderIntoDocument(<YourComponent/>);
        });


...

Expected Behavior

Component should render as it does w/o using withRouter. Unit test should be able to inspect the rendered component and assert things about refs, simulate clicks, etc...

Actual Behavior

Component is not actually rendered into the document. Accessing refs, finding elements in the document, etc... return undefined.

@timdorr
Copy link
Member

timdorr commented Jul 20, 2016

Are you running this in a browser with something like Karma? renderIntoDocument doesn't work without a DOM.

@taion
Copy link
Contributor

taion commented Jul 20, 2016

The instance renders just fine. You can't access the refs because that's just how HoCs work.

@taion taion closed this as completed Jul 20, 2016
@jrobison153
Copy link
Author

Seems like the recommendation of using the HOC is incompatible with some standard unit testing practices and patterns. I can switch to normal react contexts and be just fine then. Do you guys have test patterns for unit testing HOC wrapped components?

@taion
Copy link
Contributor

taion commented Jul 21, 2016

It's the same caveat as with @connect in React Redux.

Consider using a library like teaspoon or Enzyme that let you do testing without the use of refs. Those are essentially modern best practices.

@jrobison153
Copy link
Author

Cheers. Yes I've been looking at enzyme. Time to make the switch...

@timdorr
Copy link
Member

timdorr commented Jul 21, 2016

@taion React Redux actually does make it available on a getWrappedInstance() method, but not via refs. I don't think we need to do the same (the scope of connect's HoC is drastically different), but we can consider it if it turns out to be important to many people.

@taion
Copy link
Contributor

taion commented Jul 21, 2016

I think we should add withRef, probably. I just mean that in neither case can you access the refs from the wrapped component instance directly.

@jrobison153
Copy link
Author

jrobison153 commented Aug 10, 2016

Circling back on this topic. I did indeed migrate to enzyme + chai-enzyme, overall a good move as it made testing much cleaner. On the downside though shallow rendering exhibits similar issues as to what I was experiencing with TestUtils in the opening post. It seem that the HOC is affecting the public, maybe even private, API of the react component and related classes. An HOC is just an implementation/variation of the classic decorator/wrapper/adapter pattern, I would expect the original API of the decorated module to be left intact otherwise composing an application of many disparate libraries that depend on the API integrity will be impossible.

I pulled together a tiny project that demonstrates the issues https:/jrobison153/withRouterUnitTesting

@taion
Copy link
Contributor

taion commented Aug 10, 2016

Well the point of shallow rendering is that it only goes one level deep, so if you render something with an HoC, you would get the HoC. Again that's just how HoCs work. We'd invite a PR that adds Redux-style withRef, but otherwise React doesn't give a way around HoCs doing their thing right now.

@jrobison153
Copy link
Author

Thanks for making that connection for me, I neglected the fact that the HOC created a nested render (the obvious is always so obvious in hindsight). Swapping to a full (deep) render everything works as expected.

Sounds like for the indefinite future use of withRouter and shallow rendering provided by any library will be incompatible.

Cheers!

@Bigguy34
Copy link
Contributor

Bigguy34 commented Aug 18, 2016

Are you thinking something like this withRouter({withRef:true})(MyComponentThatNeedsRouter) then? Because I am pretty close, but I think we are going to need to write a codemod script to do the upgrade. Thoughts?

@taion
Copy link
Contributor

taion commented Aug 18, 2016

We're not deprecating context.router by any means. You can continue to use that if you want to. It's just that e.g. if React ever changes the context API, necessarily you'd have to change your code there.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants