cfaester / enzyme-adapter-react-18

MIT License
38 stars 12 forks source link

Error thrown when using act in unmounted component #2

Closed MonkeyDo closed 1 year ago

MonkeyDo commented 1 year ago

Hello!

I'm hitting an issue in some jest+enzyme tests I'm running. It looks like if a component instance is unmounted, this line will throw an error when wrapping an action with act(…): https://github.com/cfaester/enzyme-adapter-react-18/blob/main/src/ReactEighteenAdapter.ts#L475

The error in question:

TypeError: Cannot read properties of null (reading '_reactInternals')

      86 |     await act(() => {
    > 87 |       wrapper = mount(
         |                      ^
      […]

      at node_modules/@cfaester/enzyme-adapter-react-18/dist/ReactEighteenAdapter.js:392:121
      at act (node_modules/react/cjs/react.development.js:2512:16)
      at Object.getNode (node_modules/@cfaester/enzyme-adapter-react-18/dist/ReactEighteenAdapter.js:388:17)
      at new ReactWrapper (node_modules/enzyme/src/ReactWrapper.js:117:44)
      at mount (node_modules/enzyme/src/mount.js:10:10)
      […]
      at act (node_modules/react/cjs/react.development.js:2512:16)

Supposedly, this should be something like return null instead? Otherwise, in the line below instance._reactInternals will be executed and will throw an error if instance is null or undefined, making the previous check for !instance useless.

raualvron commented 1 year ago

@MonkeyDo I'm facing the same issue. Did you find a solution for that?

MonkeyDo commented 1 year ago

@raualvron Yes, I did in the end. The trick was to make sure that the mounted component is unmounted and cleared after each test.

I opted for reusing a wrapper variable and calling wrapper.unmount() in afterEach then setting the wrapper variable to undefined in beforeEach. See for example: https://github.com/metabrainz/listenbrainz-server/blob/51ddbf37c6e7351f34c5b66dd35fce74322933a4/frontend/js/tests/recommendations/Recommendations.test.tsx#L72-L88

The other option is to call wrapper.unmount() at the end of each test like so: https://github.com/metabrainz/listenbrainz-server/blob/51ddbf37c6e7351f34c5b66dd35fce74322933a4/frontend/js/tests/listens/ListenFeedbackComponent.test.tsx#L73

sienic commented 1 year ago

I think this is instead related to https://github.com/cfaester/enzyme-adapter-react-18/pull/6

cfaester commented 1 year ago

@MonkeyDo @raualvron Hey guys. 0.7.0 will be landing on npm in a few minutes. Would you be kind to check if that fixed your issues?

MonkeyDo commented 1 year ago

Thanks for the tip! Testing the new version, I echo the comment here: I am not seeing the _reactInternals issue anymore, so that issue is solved, thanks a lot !

However I am seeing some new test failures, all of them related to the absence of a mounted wrapper where one is expected (and of course I'm happy to open a separate issue for this):

Method “instance” is meant to be run on 1 node. 0 found instead.

      638 |         context: GlobalContextMock,
      639 |       });
    > 640 |       const instance = wrapper.instance();

      at ReactWrapper.single (node_modules/enzyme/src/ReactWrapper.js:1168:13)
      at ReactWrapper.instance (node_modules/enzyme/src/ReactWrapper.js:242:17)

For reference, here's the line above where i just mount the component:

it("sets URL parameters", () => {
      const wrapper = mount<UserEntityChart>(<UserEntityChart {...props} />, {
        context: GlobalContextMock,
      });
      const instance = wrapper.instance(); // <- this is throwing an error

https://github.com/metabrainz/listenbrainz-server/blob/929ca864012018d178b61dbcc1dad8873c4297b5/frontend/js/tests/stats/UserEntityChart.test.tsx#L759-L763

Upon closer inspection, the failures are consistent and consistently in the same test files. The one mentioned above uses describe.each, so I suspect it could be related somehow to how the tests are run or the order thereof.

describe.each([
  ["User Stats", userProps],
  ["Sitewide Stats", sitewideProps],
])("%s", (name, props) => {

https://github.com/metabrainz/listenbrainz-server/blob/929ca864012018d178b61dbcc1dad8873c4297b5/frontend/js/tests/stats/UserEntityChart.test.tsx#L53-L56

cfaester commented 1 year ago

@MonkeyDo Oh that is an interesting error! Your class should definitely have an instance you can assert on. Great follow-up too, I'll see if I can get around to testing your project this week.

cfaester commented 1 year ago

@MonkeyDo Hi. Please email hello@cfaester.dk. I spent a few hours with your project, and I think we should move the discussion off of a Github issue.

Closing this one for now.