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

v4 link test #4227

Closed
BigDadBear opened this issue Nov 28, 2016 · 15 comments
Closed

v4 link test #4227

BigDadBear opened this issue Nov 28, 2016 · 15 comments

Comments

@BigDadBear
Copy link

Using v4.0.0-alpha.6

Shallow rendering tests (run by jest) using Link are failing after alpha 5:

 Warning: Failed context type: The context `router` is marked as required in `Link`, but its value is `undefined`.
          in Link (at index.js:30)
          ...

Any advice on how to work around this?

@pshrmn
Copy link
Contributor

pshrmn commented Nov 28, 2016

Can you include the code for your test? I'm not sure what you need to be testing a <Link> for.

@BigDadBear
Copy link
Author

Of course, sorry. Here's a very simplified example.

import React from 'react'
import ReactDOM from 'react-dom'
import { Link } from 'react-router'

const HomeLink = () => (
  <Link to='/'>Home</Link>
)

it('renders the HomeLink', () => {
  const div = document.createElement('div')
  ReactDOM.render(<HomeLink />, div)
})

It's just a smoke test to make sure that a component can render, but I find them very useful.

@pshrmn
Copy link
Contributor

pshrmn commented Nov 28, 2016

In the unit tests a <MemoryRouter> is used to wrap a <Link> so that it has a router context.

@BigDadBear
Copy link
Author

I'll give that a shot! Thanks for the lightning-fast response time.

@BigDadBear
Copy link
Author

Did the trick, thanks a lot :)

@indiesquidge
Copy link

indiesquidge commented Feb 23, 2017

Here's an updated link to the unit tests for anyone coming across this now.

New new updated link 😅

@indiesquidge
Copy link

indiesquidge commented Feb 23, 2017

@pshrmn, I think I'm confused as to how this scales.

Do we have to wrap every component that renders—either directly or indirectly—<Link> component(s) throughout our whole app component tree when running a smoke test? That seems kind of...tedious...unless I'm missing something. I'm pretty new to testing components, so forgive my ignorance.

@BigDadBear
Copy link
Author

BigDadBear commented Feb 23, 2017

@indiesquidge That's been my experience. I ended up making a wrapper that I just reuse whenever I'm rendering router components, something like:

import { MemoryRouter } from 'react-router-dom'

const TestWrapper = (component) => <MemoryRouter>{component}</MemoryRouter>

export { TestWrapper }

----------------------

import React from 'react'
import ReactDOM from 'react-dom'
import { TestWrapper } from '...'

import { ThingToTest } from '...'

it('renders the ThingToTest', () => {
  const div = document.createElement('div')
  ReactDOM.render(TestWrapper(<ThingToTest />), div)
})

If you're using something like Redux you can even set up a store/provider for testing at this point.

@eliihen
Copy link

eliihen commented Mar 14, 2017

So this means shallow rendering in tests is out as long as you're using a <Link /> in your component? I hope I'm missing something here, as I would very much like to preserve the ability to shallow render. I tried to create a workaround to stop <Link /> from throwing, but that didn't go too well.

import React from 'react';
import * as Enzyme from 'enzyme';
import { MemoryRouter } from 'react-router-dom';

export const shallow = component => Enzyme.shallow(
  <MemoryRouter>
    {component}
  </MemoryRouter>
);

That results in this failing:

import React from 'react';
import { Link } from 'react-router-dom';
import { shallow } from 'test-utils/enzyme';

const Component = () => <Link to="foo" />;

it('should render the with a <Link />', () => {
  const wrapper = shallow(<Component />);
  console.log('wrapper.debug()', wrapper.debug());
  // <Router history={{...}}>
  //   <Component />
  // </Router>

  const link = wrapper.find('Link');
  expect(link).toBePresent();
  // Expected "[empty set].toBePresent()" results to contain at least one node, instead found none.
});

@BigDadBear
Copy link
Author

@esphen In my experience yes, you do have to do a full mount 😞

@pshrmn
Copy link
Contributor

pshrmn commented Mar 14, 2017

The reason for doing tests inside of a <MemoryRouter> is to get access to the context.router.

If you are using something like Enzyme, you can provide an options.context to the shallow/mount call to provide it with your own context. This, however, means that you have to generate a pseudo context that mirrors the layout of React Router's context.router object.

I wanted to experiment with Enzyme in my rrc package, so in there I wrote a little function that does this:
https://github.com/pshrmn/rrc/blob/master/src/__tests__/testContext.js

You can see its usage here:
https://github.com/pshrmn/rrc/blob/master/src/__tests__/Status.spec.js

It seemed to work well for me, but I still had to use mount instead of shallow for a number of components, which seemed to partially defeat the purpose.

@eliihen
Copy link

eliihen commented Mar 14, 2017

Hi @pshrmn, nice to hear there is a workaround. I'll try it out tomorrow.

I'm interested to hear, though: How do you feel about React Router falling back to rendering a placeholder if context is not available? In react-router-dom there is only one place on the initial render where context is required. It wouldn't be too hard to change it to support this case and save a lot of developers a fair bit of headache.

For example:

    let href;
    if (this.context.router) {
      href = this.context.router.history.createHref(
        typeof to === 'string' ? { pathname: to } : to
      );
    } else {
      href = (to && to.pathname) || to;
    }

@pshrmn
Copy link
Contributor

pshrmn commented Mar 14, 2017

Just published react-router-test-context, which is my above code that is slightly more complete and with tests.

I think that in most cases, rendering inside of a <MemoryRouter> is better in the sense that all of React Router's location-aware components expect to be rendered inside of a router.

That said, I think that in some situations this could be useful to setup an exact context object without having to render a number of <Route>s.

@esphen I think that at some point (I don't remember if it was v2/3 or v4 alpha), something along those lines was included. The conclusion was that it didn't make sense to include because the <Link> isn't supposed to work when context.router does not exist.

@eliihen
Copy link

eliihen commented Mar 14, 2017

@pshrmn

Just published react-router-test-context

Awesome, well done! That was super quick 😄

Just one more point before I leave this issue:

I'd be okay with <Link /> not working correctly in my tests without the proper setup, but throwing seems a bit excessive. Honestly I don't want to be thinking about React Router all until I'm actually writing a test for React Router integration.

As it is now React Router keeps pressing the issue by having to pass context on every shallow render and wrapping every other component in <MemoryRouter>. Imagine if redux did that, and there was no way to export an unconnected component.

It would be great for me as a developer, and you as a maintainer to make it "just work". Less confusion is generally a good thing, no?

@levino
Copy link

levino commented Jun 23, 2017

I agree with @esphen, there should be some silently degraded version of the link when testing. Is there any more worke being done in this?

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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

5 participants