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

[EuiCodeBlock] Cannot be used in Jest Snapshots when language is specified #2242

Closed
clintandrewhall opened this issue Aug 21, 2019 · 7 comments
Labels
⚠️ needs validation For bugs that need confirmation as to whether they're reproducible

Comments

@clintandrewhall
Copy link
Contributor

Jest Snapshot tests that include components that use EUICodeBlock fail with the following error:

TypeError: Cannot read property 'className' of null
      at blockLanguage (../node_modules/highlight.js/lib/highlight.js:76:25)
      at Object.highlightBlock (../node_modules/highlight.js/lib/highlight.js:618:20)
      at EuiCodeBlockImpl.highlightBlock [as highlight] (../node_modules/@elastic/eui/lib/components/code/_code_block.js:101:28)
      at EuiCodeBlockImpl.highlight [as componentDidMount] (../node_modules/@elastic/eui/lib/components/code/_code_block.js:141:12)
      at commitLifeCycles (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9251:22)

To Reproduce

  1. Create a test file with the following:
import React from 'react';
import renderer from 'react-test-renderer';
import { EuiCodeBlock } from '@elastic/eui';

const REPORTING_CONFIG = `xpack.reporting:
  enabled: true
  capture.browser.type: chromium`;

it('renders correctly', () => {
  const tree = renderer
    .create(<EuiCodeBlock language="yml">{REPORTING_CONFIG}</EuiCodeBlock>)
    .toJSON();
  expect(tree).toMatchSnapshot();
});
  1. Run the test, observe error.
  2. Remove language="yml".
  3. Run the test, see it passes.
@thompsongl
Copy link
Contributor

Thanks for taking this one @chandlerprall

I looked very briefly earlier and found that there are no problems when using jest.render. It only errors with react-test-renderer

@chandlerprall
Copy link
Contributor

chandlerprall commented Aug 21, 2019

By design, components which require refs do not work with react-test-renderer as it doesn't create DOM elements (facebook/react#7740). If you really want to make them work, there's a workaround where you can provide a createNodeMock function (https://reactjs.org/docs/test-renderer.html#ideas). I've thrown together the following snippet which appears to make the test pass as desired (assuming a jest or other jsdom-enabled environment):

// attribute map taken from react-dom
const reactAttributeNameMap = {
  acceptCharset: 'accept-charset',
  className: 'class',
  htmlFor: 'for',
  httpEquiv: 'http-equiv',
};
function createNodeMock({ type, props }) {
  const element = document.createElement(type);
  const propNames = Object.keys(props);
  propNames.forEach(name => {
    const attributeName = reactAttributeNameMap.hasOwnProperty(name) ? reactAttributeNameMap[name] : name;
    element.setAttribute(attributeName, props[name]);
  });
  return element;
}

// ...
    renderer.create(
      <EuiCodeBlock language="yml">var hello = 'world';</EuiCodeBlock>,
      { createNodeMock }
    );
// ...

@clintandrewhall
Copy link
Contributor Author

The problem is that this requires an internal knowledge of the component in question. I can add a mock of some kind to our shots test which would mean I'd simply know that something in the tree uses a ref.

// Disabling this test due to https://github.com/elastic/eui/issues/2242
jest.mock('public/components/workpad_header/workpad_export/__examples__/disabled_panel.examples', () => {
  return <div>Disabled Panel</div>
})

Is it possible to holistically approach a fix, where I wouldn't need to understand this?

@chandlerprall
Copy link
Contributor

The createNodeMock implementation I provided will support any basic ref usage, it is not tailored for this particular case. One thing it explicitly does not do is render & mount the children into the mocked-out jsdom element, so there may be unexpected results or errors when used with other components, if they rely heavily on a known DOM structure to exist.

@clintandrewhall
Copy link
Contributor Author

While I'd prefer to see a mock included as part of eui, perhaps using node module mocks, it turns out there are two easy ways to mitigate this in our case:

Option 1: Directly mock the component that uses a ref

jest.mock('@elastic/eui/lib/components/code/_code_block', () => ({
  EuiCodeBlockImpl: 'code',
  propTypes: {}, // Necessary; used by composing components
}));

Result

<code
  className="canvasWorkpadExport__reportingConfig"
  fontSize="s"
  inline={false}
  language="yml"
  paddingSize="s"
>
  xpack.reporting:
enabled: true
capture.browser.type: chromium
</code>

Option 2: Use Storyshots createNodeMock

initStoryshots({
  configPath: path.resolve(__dirname, './../.storybook'),
  test: multiSnapshotWithOptions({
    createNodeMock: element => {
      if (element.type === 'code') {
        return document.createElement('code');
      }
    },
  }),
});

Result

<div
  className="euiCodeBlock euiCodeBlock--fontSmall euiCodeBlock--paddingSmall canvasWorkpadExport__reportingConfig"
  style={Object {}}
>
  <pre
    className="euiCodeBlock__pre"
  >
    <code
      className="euiCodeBlock__code yml"
    >
      xpack.reporting:
enabled: true
capture.browser.type: chromium
    </code>
  </pre>
</div>

@chandlerprall chandlerprall removed their assignment Oct 28, 2019
@cchaos cchaos changed the title EUICodeBlock cannot be used in Jest Snapshots when language is specificed [EuiCodeBlock] Cannot be used in Jest Snapshots when language is specified Sep 20, 2020
@chandlerprall
Copy link
Contributor

This was likely resolved by replacing highlight.js in #4638

@chandlerprall chandlerprall added ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible and removed assign:engineer bug labels Jun 29, 2021
@thompsongl
Copy link
Contributor

thompsongl commented Aug 23, 2021

This is resolved by the switch to prism.js and the update to how the fallback language is handled. Feel free to re-open if I'm mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ needs validation For bugs that need confirmation as to whether they're reproducible
Projects
None yet
Development

No branches or pull requests

3 participants