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

Reconsider “unnecessary” whitespace in HTMLElement plugin #3825

Closed
pedrottimark opened this issue Jun 14, 2017 · 0 comments · Fixed by #4275
Closed

Reconsider “unnecessary” whitespace in HTMLElement plugin #3825

pedrottimark opened this issue Jun 14, 2017 · 0 comments · Fixed by #4275

Comments

@pedrottimark
Copy link
Contributor

pedrottimark commented Jun 14, 2017

Do you want to request a feature or report a bug?

Report what could be interpreted as a bug in #3479 /cc @thymikee my friend :)

What is the current behavior?

The following React elements include a space between the span elements:

<p>
  <span>Preceding</span> <span>following.</span>
</p>

It affects the text that browsers display: Preceding following. instead of Precedingfollowing.

ReactElement and ReactTestComponent plugins render it:

exports[`white space in ReactElement plugin 1`] = `
<p>
  <span>
    Preceding
  </span>
   
  <span>
    following.
  </span>
</p>
`;

exports[`white space in ReactTestComponent plugin 1`] = `
<p>
  <span>
    Preceding
  </span>
   
  <span>
    following.
  </span>
</p>
`;

If I inspect in a browser what is rendered, and then adapt a test for it:

test('white space in HTMLElement plugin', () => {
  const parent = document.createElement('p');
  parent.innerHTML = '<span>Preceding</span><!-- react-text: 3 --> <!-- /react-text --><span>following.</span>';
  expect(parent).toMatchSnapshot();
});

The text node which consists only of white space is filtered out.

exports[`white space in HTMLElement plugin 1`] = `
<p>
  <span>
    Preceding
  </span>
  <!-- react-text: 3 -->
  <!-- /react-text -->
  <span>
    following.
  </span>
</p>
`;

React 15.6.0 renders comments but 16.0.0-alpha.13 renders a space not surrounded by comments.

The result is similar if I test an HTML element from findDOMNode in a React component, which I think is the original problem in #2146 although the goal there was not a snapshot test.

What is the expected behavior?

That is the question.

  • If the most common use is output to console when test expects null but receives an element, more concise might be more better.
  • If the most common source of the HTML element is React component render, and any testers ever do take a snapshot, then whitespace more consistent with the other markup plugins (and even more, what React renders) seems better.

What brought me to this issue

  1. Except for the filter method at https://github.com/facebook/jest/blob/master/packages/pretty-format/src/plugins/HTMLElement.js#L60 we can factor out an identical printChildren function for three plugins that serialize markup:

    • HTMLElement
    • ReactElement
    • ReactTestComponent
  2. EDIT: After we decide about the space-reducing logic in print an 2 smaller inconsistency [second which does not directly affect Jest testing] is whether the return for text node:

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

Jest 20
React 15.6.0 or 16.0.0-alpha.13
Node.js 8.1.0
MacOS 10.12.5

If it helps, I can create a repo on GitHub.

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

Successfully merging a pull request may close this issue.

2 participants