search

surevine / govuk-react-jsx

govuk-frontend compatible React components
https://govuk-react-jsx.netlify.app/
MIT License
3 stars 0 forks source link

Getting an error on certain components when rendered in CRA testing environment #109

Closed SamChatfield closed 3 years ago

SamChatfield commented 3 years ago

Hi there,

I'm trying to test a form component I've built that uses various govuk-react-jsx components and Formik. The form is essentially just an ErrorSummary, a DateInput, and a Button. The project's run and test scripts are using react-scripts and it all works perfectly when running with react-scripts start, but my problem comes when I am trying to unit test the form under react-scripts test using the package @testing-library/react where I get an error when the Button and ErrorSummary components are rendered.

The error message is as follows:

TypeError: _vm(...).SyntheticModule is not a constructor

      at Runtime.loadCjsAsEsm (node_modules/jest-runtime/build/index.js:657:20)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/error-summary/index.js:45:15)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)

It seems like something about the environment used by react-scripts test has an issue with the import(...) function calls in the Button and ErrorSummary components. The DateInput doesn't have an import(...) like that and is the one that is working okay.

If I remove the Button component from the form and instead create it myself using HTML and the proper govuk classes the issue disappears for the button render, and I am left with the error above on tests which submit values that fail validation and the ErrorSummary is consequently rendered.

I wonder if you have seen this issue before or have any idea what could be going on here?

andymantell commented 3 years ago

Hmm. That error doesn't ring a bell :-/

Is there any chance you could throw a repository together demonstrating the error? I can have a poke around then...

SamChatfield commented 3 years ago

Okay I think this should be a pretty barebones reproduction: https://github.com/SamChatfield/govuk-react-demo

Should be able to see that everything runs okay under npm start, but the tests error under npm test.

I've set it up so clicking the button adds an error to the state, and the ErrorSummary is conditionally rendered if this errors array in the state has something in it.

There's two tests:

I added a log in the click handler and render functions of my component to show how these are being called during the tests.

andymantell commented 3 years ago

Thanks Sam, I'll try and have a look at lunchtime

andymantell commented 3 years ago

Note to self for later:

https://jestjs.io/docs/webpack#using-with-webpack-2 (babel-plugin-dynamic-import-node)

Also - why isn't this an issue in the govuk-react-jsx tests 🤔

andymantell commented 3 years ago

@SamChatfield Can I just check - you're using nodejs 12 as indicated by the nvmrc file?

SamChatfield commented 3 years ago

Yeah that's right, and npm 6.14.6.

andymantell commented 3 years ago

Interesting. All I get when I run npm test on node12 is:

> govuk-react-demo@0.1.0 test /mnt/c/Users/andym/work/govuk-react-demo
> react-scripts test --watchAll=false

(node:2620) ExperimentalWarning: The ESM module loader is experimental.
SamChatfield commented 3 years ago

That's strange, it looks like it's not running the tests on your end then if that's the only output. Here's the full output of what I'm seeing:


> govuk-react-demo@0.1.0 test /Users/samch/_/_/_/govuk-react-demo
> react-scripts test --watchAll=false

(node:27406) ExperimentalWarning: The ESM module loader is experimental.
  console.log
    DatePicker render

      at DatePicker.render (src/components/DatePicker.jsx:25:17)

  console.log
    DatePicker render

      at DatePicker.render (src/components/DatePicker.jsx:25:17)

  console.log
    Submit clicked

      at DatePicker.handleClick (src/components/DatePicker.jsx:18:17)

  console.log
    DatePicker render

      at DatePicker.render (src/components/DatePicker.jsx:25:17)

 FAIL  src/components/DatePicker.test.jsx
  test date picker component
    ✕ should render submit button (77 ms)
    ✕ should display error summary when an error is present in state (47 ms)

  ● test date picker component › should render submit button

    TypeError: _vm(...).SyntheticModule is not a constructor

      at Runtime.loadCjsAsEsm (node_modules/jest-runtime/build/index.js:657:20)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/button/index.js:69:15)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)

  ● test date picker component › should display error summary when an error is present in state

    TypeError: _vm(...).SyntheticModule is not a constructor

      at Runtime.loadCjsAsEsm (node_modules/jest-runtime/build/index.js:657:20)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/button/index.js:69:15)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)

  ● test date picker component › should display error summary when an error is present in state

    TypeError: _vm(...).SyntheticModule is not a constructor

      at Runtime.loadCjsAsEsm (node_modules/jest-runtime/build/index.js:657:20)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/error-summary/index.js:45:15)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)

  ● test date picker component › should display error summary when an error is present in state

    TypeError: _vm(...).SyntheticModule is not a constructor

      at Runtime.loadCjsAsEsm (node_modules/jest-runtime/build/index.js:657:20)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/button/index.js:69:15)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 2 total
Snapshots:   0 total
Time:        3.676 s
Ran all test suites.
npm ERR! Test failed.  See above for more details.
andymantell commented 3 years ago

ah it's ok, I think I'd broken it whilst trying to fix it :-) I've got the errors now

andymantell commented 3 years ago

ok it hasn't yielded to lunchtime investigations. I'll think about it a bit more

SamChatfield commented 3 years ago

No worries, I appreciate you getting on to this so quickly.

andymantell commented 3 years ago

no problem. I always like to at least look quickly in case it's obvious, but in this case not :-)

SamChatfield commented 3 years ago

A little bit of extra info. I came across the node --experimental-vm-modules command line option, so I thought I'd try running the tests as NODE_OPTIONS='--experimental-vm-modules' npm test to see what would happen and those TypeError: _vm(...).SyntheticModule is not a constructor errors go away and are replaced by a single different error in the second test only:

TypeError: Cannot read property 'addEventListener' of null

      at Button.Object.<anonymous>.Button.init (node_modules/govuk-frontend/govuk/components/button/button.js:718:16)
      at _callee$ (node_modules/govuk-react-jsx/govuk/components/button/index.js:80:47)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

I don't know if there is a clue in here somewhere. I also don't know why react-scripts test would behave differently regarding dynamic imports than react-scripts start but that command line option changing the errors makes it seem like it is for some reason.

andymantell commented 3 years ago

What about switching to a newer version of node? 14 maybe?

The second error looks like this one:

https://github.com/surevine/govuk-react-jsx/issues/99

So this kinda ups the priority of fixing that one. Which is frustrating because I'm not sure how or why it's happening. But clearly I need to dig into that one a bit more... I will try and set some time aside this weekend to investigate it.

I guess in the meantime, if we can get to the bottom of your synthetic module errors that would be progress... Are you able to go to node 14?

SamChatfield commented 3 years ago

Okay so I tried switching to node v14.16.0 and now I get no output from the tests, it just says they failed without any further info:


> govuk-react-demo@0.1.0 test /Users/samch/_/_/_/govuk-react-demo
> react-scripts test --watchAll=false

 RUNS  src/components/DatePicker.test.jsx
npm ERR! Test failed.  See above for more details.
andymantell commented 3 years ago

Hmm ok, I'll keep an eye on that too but not sure what's going on there.

SamChatfield commented 3 years ago

I suspect that might also have something to do with the internals of the Button component as well because if I remove the Button and test something else in the tests such as the DateInput legend, it works under node 14.

andymantell commented 3 years ago

@SamChatfield I've fixed #99 which was causing the Cannot read property 'addEventListener' of null error but there's still something a bit odd going on. As you say, the test output is empty, which is really odd. Even on nodejs 12 for me.

Can you try doing npm install govuk-react-jsx@next and see where it leaves you? Do you have a working app at least? But broken tests...

Also in case it's useful, I have a date field component lurking in #4 which might interest you, using Formik. It's got some tests to go with it but it just needs documenting. I would hesitate before calling it "finished" but it should be a good start. You can just drop it into a Formik form like this:

<DateField
          id="datefrom"
          namePrefix="datefrom"
          items={[
            {
              name: "day",
            },
            {
              name: "month",
            },
            {
              name: "year",
            },
          ]}
        />
SamChatfield commented 3 years ago

Okay it looks like your change to the Button has fixed the problem when running with the --experimental-vm-modules flag under node 12 which is great. Also I think I've figured out how to get babel to transform the dynamic imports so that it works without the flag. I'll need to do some more testing on Monday just to double check everything is working out but it's looking promising.

Thanks so much for your help on this, I'll drop another comment on Monday just to confirm it's working when I've satisfied my paranoia.

andymantell commented 3 years ago

Ah good, glad we're making progress.

Please do let me know how it goes. And it would be amazing if you could post a quick summary of what the resolution was for future reference by me or others. Or even better update that git repo with the solution.

Thanks, have a good weekend.

SamChatfield commented 3 years ago

Yeah so it all looks like it's working now both in the demo repo and my actual project 🎉.

I've committed the fix to the demo repo for future reference. The key parts to the fix from my end were:

The above means you don't have to use the --experimental-vm-modules flag in node 12. The babel dynamic import plugin is not needed. I believe this is because babel preset-env is detecting that we are running in node via jest and thus transpiles ES modules/dynamic imports automatically (see https://babeljs.io/docs/en/babel-preset-env#modules-auto).

All of this combined with your fix for the addEventListener issue in govuk-react-jsx@5.1.0-rc1 means that everything I'm doing currently is now working perfectly including the Button component which I've put back into my main project.

Thanks once again for your help!

andymantell commented 3 years ago

Perfect, thankyou very much for the writeup. I think what I might do is add some tests to the govuk-react-jsx-examples and fold in your changes there. And stick something in the readme about test setup.

The transformIgnorePatterns is an interesting one - I wonder if I should be publishing the components in a different way. Might need to read up on that. Although they work with create-react-app out the box without needing to do any exceptions like that - I wonder why jest needs it 🤔 So many variables 🤯

Glad it's all working now anyway. Stick with that rc1 version for a second if you don't mind, until I hear back on #99 and then I'll do a proper release... (I'll leave this ticket open until I do that subsequent release)

andymantell commented 3 years ago

This has been released now as https://github.com/surevine/govuk-react-jsx/releases/tag/v5.1.0

Any more problems let me know.