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

TypeError: Cannot read properties of null (reading 'off') during mount when running tests for a wrapper component #81

Closed
AndrewOttavianoAbb opened this issue Jun 1, 2022 · 8 comments
Labels
kind: support Asking for support with something or a specific use case scope: downstream Issue in a downstream library, not in this library itself scope: integration Related to an integration, not necessarily to core (but could influence core) solution: can't repro An attempt to reproduce has been tried and failed

Comments

@AndrewOttavianoAbb
Copy link

AndrewOttavianoAbb commented Jun 1, 2022

Hello and thank you so much for your work on this project! I am creating a wrapper around the react-signature-canvas and am working on writing jest tests for it with testing-libarary/react. I am simply trying to mount it to ensure it displays properly and am receiving the following error. I'm not passing anything to it besides a ref (created with useRef); I'm not even touching canvasProps (at this point). I am rendering the parent <div /> with a data-testid so that I can grab hold of it in testing-library.

Test Code:

describe('Signature', () => {
  it('Component renders successfully', () => {
    render(<Signature SubmitComponent={() => <div />} />);
    const sigCanvas = screen.getByTestId('react-signature-canvas');

    expect(sigCanvas).toBeInTheDocument();
  });
});

(Signature is my wrapper component, this is what I'm actually rendering <SignatureCanvas ref={canvasRef} {...canvasAttributes} /> but canvasAttributes is null in my test above.)

Stack trace:
 ● Signature › Component renders successfully

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

       5 | describe('Signature', () => {
       6 |   it('Component renders successfully', () => {
    >  7 |     render(<Signature SubmitComponent={() => <div />} />);
         |     ^
       8 |     const sigCanvas = screen.getByTestId('react-signature-canvas');     
       9 |
      10 |     expect(sigCanvas).toBeInTheDocument();

      at t.r.off (../../node_modules/react-signature-canvas/build/index.js:1:3228) 
      at t.value (../../node_modules/react-signature-canvas/build/index.js:1:3771) 
      at callComponentWillUnmountWithTimer (../../node_modules/react-dom/cjs/react-dom.development.js:20413:14)
      at HTMLUnknownElement.callCallback (../../node_modules/react-dom/cjs/react-dom.development.js:3945:14)
      at HTMLUnknownElement.callTheUserObjectsOperation (../../node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30)
      at innerInvokeEventListeners (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:338:25)
      at invokeEventListeners (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:274:3)
      at HTMLUnknownElementImpl._dispatch (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:221:9)
      at HTMLUnknownElementImpl.dispatchEvent (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:94:17)
      at HTMLUnknownElement.dispatchEvent (../../node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:231:34)
      at Object.invokeGuardedCallbackDev (../../node_modules/react-dom/cjs/react-dom.development.js:3994:16)
      at invokeGuardedCallback (../../node_modules/react-dom/cjs/react-dom.development.js:4056:31)
      at safelyCallComponentWillUnmount (../../node_modules/react-dom/cjs/react-dom.development.js:20420:5)
      at commitUnmount (../../node_modules/react-dom/cjs/react-dom.development.js:20951:11)
      at commitNestedUnmounts (../../node_modules/react-dom/cjs/react-dom.development.js:21004:5)
      at unmountHostComponents (../../node_modules/react-dom/cjs/react-dom.development.js:21290:7)
      at commitDeletion (../../node_modules/react-dom/cjs/react-dom.development.js:21347:5)
      at commitMutationEffects (../../node_modules/react-dom/cjs/react-dom.development.js:23407:11)
      at HTMLUnknownElement.callCallback (../../node_modules/react-dom/cjs/react-dom.development.js:3945:14)
      at HTMLUnknownElement.callTheUserObjectsOperation (../../node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30)
      at innerInvokeEventListeners (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:338:25)
      at invokeEventListeners (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:274:3)
      at HTMLUnknownElementImpl._dispatch (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:221:9)
      at HTMLUnknownElementImpl.dispatchEvent (../../node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:94:17)
      at HTMLUnknownElement.dispatchEvent (../../node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:231:34)
      at Object.invokeGuardedCallbackDev (../../node_modules/react-dom/cjs/react-dom.development.js:3994:16)
      at invokeGuardedCallback (../../node_modules/react-dom/cjs/react-dom.development.js:4056:31)
      at commitRootImpl (../../node_modules/react-dom/cjs/react-dom.development.js:23121:9)
      at unstable_runWithPriority (../../node_modules/scheduler/cjs/scheduler.development.js:468:12)
      at runWithPriority$1 (../../node_modules/react-dom/cjs/react-dom.development.js:11276:10)
      at commitRoot (../../node_modules/react-dom/cjs/react-dom.development.js:22990:3)
      at performSyncWorkOnRoot (../../node_modules/react-dom/cjs/react-dom.development.js:22329:3)
      at ../../node_modules/react-dom/cjs/react-dom.development.js:11327:26        
      at unstable_runWithPriority (../../node_modules/scheduler/cjs/scheduler.development.js:468:12)
      at runWithPriority$1 (../../node_modules/react-dom/cjs/react-dom.development.js:11276:10)
      at flushSyncCallbackQueueImpl (../../node_modules/react-dom/cjs/react-dom.development.js:11322:9)
      at flushSyncCallbackQueue (../../node_modules/react-dom/cjs/react-dom.development.js:11309:3)
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:22387:7)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14)
      at render (../../node_modules/@testing-library/react/dist/pure.js:97:26)     
      at Object.<anonymous> (src/lib/signature/Signature.spec.tsx:7:5)
      at TestScheduler.scheduleTests (../../node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (../../node_modules/@jest/core/build/runJest.js:404:19)

Rendering in the browser seems to work. Any suggestions as to what I'm doing wrong or is this a bug?

@agilgur5 agilgur5 changed the title TypeError: Cannot read properties of null (reading 'off') TypeError: Cannot read properties of null (reading 'off') during mount when running tests for a wrapper component Jun 2, 2022
@agilgur5 agilgur5 added kind: support Asking for support with something or a specific use case scope: integration Related to an integration, not necessarily to core (but could influence core) problem: no repro No reproduction was provided (and have not tried to repro without one) labels Jun 2, 2022
@agilgur5
Copy link
Owner

agilgur5 commented Jun 2, 2022

I am creating a wrapper around the react-signature-canvas and am working on writing jest tests for it with testing-libarary/react

Wrapper components are definitely encouraged since this is an unopinionated and unstyled component! Everyone's got their own UI components, UX flows, and CSS styles, so I think composition for a more specific use-case is good separation of concerns (while this component handles the generic use-case).
When I originally built this component years ago, I also had an internal wrapper around it at my company, with buttons, a modal, react-bootstrap integration, etc.
Also tested components great too 👍

Rendering in the browser seems to work. Any suggestions as to what I'm doing wrong or is this a bug?

Hard to tell without a reproduction as there's a number of moving pieces here.
I've never seen this error before and a few pieces of your issue hint that this isn't a bug:

  1. The stack trace shows that is being hit during unmounting actually. I'm not sure why that's happening, but componentWillUnmount is being called. That calls the off method to remove the event listeners.
    In the current version of react-signature-canvas, the underlying signature_pad ref starts as null, so from the error message, it sounds like the internal ref is null during your mount.
    The next version of react-signature-canvas (the main branch) is written in TS and because of the strict typing, actually has an error check if the ref is null. It's a rare case, but it usually means you're calling a method during a mount/unmount (sounds related, right?). Even in that version though, it would still error, but it would give a more specific (and hopefully more helpful) error from react-signature-canvas itself.

  2. You mentioned that rendering in browser works, so it may very well be your test set-up. Internally, the tests I wrote for this component use jsdom and an additional window-resizeto polyfill (for mocking the resize functionality). If there's no DOM, a DOM ref wouldn't exist, so that could very well break a bunch of things like this.
    So I'm not sure what your jest.config.js looks like, but my suspicion would be that the test set-up is missing some configuration.

@agilgur5 agilgur5 added the scope: downstream Issue in a downstream library, not in this library itself label Jun 2, 2022
@agilgur5
Copy link
Owner

agilgur5 commented Jun 2, 2022

Looking at the stack trace a bit more, jsdom does pop up so I'm assuming you are using jsdom?
Also not sure which version of React this is on, as React 18 is currently untested (c.f. #76, which also discusses the need to migrate to RTL from Enzyme -- this may be related given that you're using RTL)

EDIT: Hmmm... it is possible you're running into the exact situation discussed in the TS rewrite review: #42 (comment) .
As I mentioned there, it's possible that doing nothing (i.e. catching the error and doing a no-op instead) when the ref is null is more appropriate than throwing an error; the crux there was if such a situation were proper user behavior or erroneous behavior (where throwing an error is indeed appropriate). That rewrite is non-breaking though, as it would error out regardless.
You're literally just mounting it, so err, that doesn't seem like erroneous behavior (😅 ), but as it only happens during testing, it still may be a testing set-up issue or a bug upstream in one of the testing libraries. Especially as this hasn't shown up in the many years this has existed until now

@AndrewOttavianoAbb
Copy link
Author

@agilgur5 thank you for the quick response! I am using jsdom but it is the RTL wrapper around it import '@testing-library/jest-dom'; I am still on react 17, so that shouldn't be an issue.

@agilgur5
Copy link
Owner

agilgur5 commented Jun 2, 2022

I am using jsdom but it is the RTL wrapper around it import '@testing-library/jest-dom';

Mmm @testing-library/jest-dom isn't a wrapper for jsdom, it's a set of DOM matchers for Jest. It doesn't depend on jsdom either.

A minimal reproduction would help here. Especially your jest.config.js, package.json, and your component's render code. The last part may be causing an error or something that's making it unmount in the testing environment.

@AndrewOttavianoAbb
Copy link
Author

I am working on getting a repro ready to go. Unfortunately, the project I'm working on is closed source. I am trying to get a codesandbox example but am running into with fillStyle which I've read can be fixed w/jest-canvas-mock but am not getting anywhere with that.

Here it is if you want to give it a try: https://codesandbox.io/s/react-17-forked-y43gl3?file=/src/tests/Signature.spec.js

@agilgur5
Copy link
Owner

agilgur5 commented Jun 2, 2022

I am trying to get a codesandbox example but am running into with fillStyle which I've read can be fixed w/jest-canvas-mock but am not getting anywhere with that.

Yea this is probably the crux of the issue -- the canvas implementation isn't working.
react-signature-canvas's internal tests use node-canvas, which jsdom supports automatically if detected.

Here it is if you want to give it a try: https://codesandbox.io/s/react-17-forked-y43gl3?file=/src/tests/Signature.spec.js

You're using CRA for this CodeSandbox, so you may very well be running into hustcc/jest-canvas-mock#72 .
I couldn't get it to work in CodeSandbox, and that might be due to requiring native binaries like node-canvas etc, or due to CRA issues. Had some issues running in a node template (couldn't get things to install), but that may make more sense than the CRA template since Jest runs via Node etc.

I did create a more minimal StackBlitz project for this instead, and the tests pass fine there: https://stackblitz.com/edit/rsc-issue-81?file=index.spec.js .
So I think the issue is within some part of your environment, as a minimal example succeeds.

@agilgur5 agilgur5 closed this as completed Jun 2, 2022
@agilgur5 agilgur5 added solution: can't repro An attempt to reproduce has been tried and failed and removed problem: no repro No reproduction was provided (and have not tried to repro without one) labels Jun 2, 2022
@AndrewOttavianoAbb
Copy link
Author

Switching to node-canvas fixed that test. Found the same suggestion here.

@agilgur5 thank you for your responsiveness and willingness to help!

@agilgur5
Copy link
Owner

agilgur5 commented Jun 4, 2022

Fun fact, apparently I ran into the fillStyle issue as well when I was first creating the test suite 3 years ago in #33 (comment) .
Using compatible versions of node-canvas and jsdom resolved it there per that issue.

Repository owner locked as resolved and limited conversation to collaborators Jun 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: support Asking for support with something or a specific use case scope: downstream Issue in a downstream library, not in this library itself scope: integration Related to an integration, not necessarily to core (but could influence core) solution: can't repro An attempt to reproduce has been tried and failed
Projects
None yet
Development

No branches or pull requests

2 participants