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

Style wrapping in 1.0 breaks Enzyme’s text() assertions #11927

Closed
2 tasks done
tdd opened this issue Jun 20, 2018 · 6 comments
Closed
2 tasks done

Style wrapping in 1.0 breaks Enzyme’s text() assertions #11927

tdd opened this issue Jun 20, 2018 · 6 comments

Comments

@tdd
Copy link

tdd commented Jun 20, 2018

When using shallow rendering, Enzyme lets us easily use text() or contain.text() assertions to check on textual content that is the entire set of child nodes for the node being tested. Material-UI's Typography component seems to break it.

For instance, if my component looked like this with Material UI pre-1.0:

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Pre-1.0, we didn't need Typography all over the place, font styling cascaded. So my component would have looked like this:

    <div className="goal">
      <div className="summary">
        <h2>{name}</h2>
        <Gauge value={progress} max={target} />
        <small>{progress} {units} sur {target}</small>
      </div>
      <div className="cta">{adderComponent}</div>
    </div>

And I was able to smoothly do:

const wrapper = shallow(<GoalTrackerWidget goal={goal} progress={progress} />)
expect(wrapper.find('h2')).to.have.text(goal.name)
expect(wrapper).to.contain.text(`${progress} ${goal.units} sur ${goal.target}`)

In 1.0, my component has to look like this:

    <div className="goal">
      <div className="summary">
        <Typography variant="title" component="h2">{name}</Typography>
        <Gauge value={progress} max={target} />
        <Typography component="small">{progress} {units} sur {target}</Typography>
      </div>
      <div className="cta">{adderComponent}</div>
    </div>

And I would absolutely expect to be able to do this:

const wrapper = shallow(<GoalTrackerWidget goal={goal} progress={progress} />)
expect(wrapper.find('Typography[component="h2"]')).to.have.text(goal.name)
expect(wrapper).to.contain.text(`${progress} ${goal.units} sur ${goal.target}`)

If necessary, by using shallow = createShallow({ untilSelector: 'WithStyles' }) instead of Enzyme's built-in shallow.

Current Behavior

However much I try, tweaking createShallow options (adding dive: true or not, etc.), when I stumble on a Typography component, its stated text is always <WithStyles(Typography) />, regardless of actual content. It nevers provides its child text as part of its shallow rendering. I have to resort to some klunky code much like the one found in your own test suites, like:

expect(wrapper.find('.name').childAt(0)).to.have.text(goal.name)
expect(wrapper.find('.details').children().map((c) => c.text()).join('')).to.equal(
  `${progress} ${goal.units} sur ${goal.target}`
)

(that latter expression doesn't even work properly, as a shallow wrapper node over a number doesn't yield the proper text, apparently)

Steps to Reproduce (for bugs)

  1. Go to this Codesandbox
  2. Wait for full module transpiling, and tests to run
  3. Check the console tab: it displays the debug() view of the Typography node. You can clearly see it has a single text child with "Material-UI".
  4. Check the test output: the wrapper's text() result is a fixed, context-free <WithStyles(Typography) />, instead of the node’s actual text contents.

Context

This completely impedes my ability to test the presence of text contents / fragments inside my React components.

Your Environment

Tech Version
Material-UI v1.2.1
React v16.4.1
Enzyme v4.19.1
Jest v23.1.0
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 20, 2018

@tdd Thanks for opening this issue. Digging more into it, I start to believe it's enzyme related. The behavior difference v0.x and v1.x can be expected. The large majorities of the v1.x component are now exported with a withStyles() higher-order component. It's changing the enzyme behavior. Using createShallow won't help.

This comment might give us some light 💡: enzymejs/enzyme#692 (comment). The best solution I can think of is:

-expect(typoElement.text()).toEqual("Material-UI");
+expect(typoElement.children().text()).toEqual("Material-UI");

https://codesandbox.io/s/q3w4r2o89j

@tdd
Copy link
Author

tdd commented Jun 21, 2018

Hey @oliviertassinari that is correct. There's actually another bug I just submitted on the incorrect rendering by text() of a node that consists solely of an interpolated zero number ({0} in JSX), that turns into an empty String (it looks as though it's killed by the children() listing, too!).

This trick works when your single child is the text node (in which case children() and childAt(0) are the same thing), but text() cannot be called on a multi-element children() wrapper—hence the call for a childrenText() helper that would do a deep traversal and concatenation of the vdom tree, much like HTML's innerText property.

I would advocate that string interpolation for child contents not be done with JSX:

// Not a good idea, test-wise and vDOM-wise
return (
  <Typography…>
    {progress} {units} out of {target}
  </Typography>
)

…but rather be done using ES Template Strings in a single text child node:

// More practical in all other aspects
return (
  <Typography…>
    {`${progress} ${units} out of ${target}`}
  </Typography>
)

At least this made it much easier for me to test and looks much better in, say, Jest snapshots.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 21, 2018

For some reason this works too:

        <Typography variant="display1" gutterBottom>
          {"Material"}  {"UI"}
        </Typography>
    const typoElement = wrapper.find(
      'WithStyles(Typography)[variant="display1"]'
    );
    console.log(typoElement.debug());
    expect(
      typoElement
        .dive()
        .dive()
        .text()
    ).toEqual("Material UI");

Maybe using the mount API in this case would make more sense.

@tdd
Copy link
Author

tdd commented Jun 21, 2018

Yes, I think that's because there are no inline whitespace elements between the interpolated elements.

@oliviertassinari
Copy link
Member

@tdd
Copy link
Author

tdd commented Jun 21, 2018

That is so weird, it b0rks on my own code when I get multiple children like this…?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants