mozmorris / react-webcam

Webcam component
https://codepen.io/mozmorris/pen/JLZdoP
MIT License
1.65k stars 281 forks source link

Jest tests show unstable_flushDiscreteUpdates warning in 6.0.0 #306

Open shiraze opened 3 years ago

shiraze commented 3 years ago

Is anyone else seeing a warning similar to the following when upgrading from v5.2.4 to v6.0.0?

PASS src/components/imageCaptureDialog/imageCaptureControlMount.test.jsx ● Console

console.error
  Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.
      in video (created by Webcam)
      in Webcam (at imageCaptureControl.jsx:163)
      in div (at imageCaptureControl.jsx:162)
      in ForwardRef (created by WrapperComponent)
      in WrapperComponent

  at printWarning (node_modules/react-dom/cjs/react-dom.development.js:88:30)
  at error (node_modules/react-dom/cjs/react-dom.development.js:60:5)
  at flushDiscreteUpdates (node_modules/react-dom/cjs/react-dom.development.js:21817:9)
  at flushDiscreteUpdatesIfNeeded (node_modules/react-dom/cjs/react-dom.development.js:829:5)
  at dispatchDiscreteEvent (node_modules/react-dom/cjs/react-dom.development.js:4167:3)
  at HTMLVideoElement.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)

Code/tests remain unchanged, but worked fine with v5.2.4

mateusdearaujo commented 3 years ago

same error here, @shiraze any solution for this problem?

worked fine with v5.2.4 too.

shiraze commented 3 years ago

I'm currently using Enzyme, but we're in the process of moving some tests to Testing-library. If that helps, I'll update this thread

sorinpav commented 2 years ago

@shiraze we're still getting the exact warning while using React Testing Library as well, so I would say it's not due to the testing library used.

Anyone encountered this so far and has a solution?

shiraze commented 2 years ago

We got this working in Enzyme by adding the following mock:

    Object.defineProperty(HTMLMediaElement.prototype, "muted", {
      set: () => {},
    });
brycesenz commented 1 year ago

@shiraze - really appreciate that snippet! I set me down a rabbit hole for a few days, but posting our solution here. Essentially what we wanted for our testing was a way to mock a dummy screenshot value. This ended up meaning we had to mock a few HTML element properties that this library uses internally, which may be too much hacking, but worked for us. We ended up writing this helper method:

const mockReactWebcamScreenshot = ({ image, height, width }) => {
  const videoHeight = height || 360;
  const videoWidth = width || 720;

  // This is necessary to avoid an `unstable_flushDiscreteUpdates` warning
  Object.defineProperty(HTMLMediaElement.prototype, 'muted', {
    set: () => {},
  });

  // Here we're mocking fake media devices so the webcam can return a screenshot
  const fakeMedia = {
    stop: () => {}
  }

  const mockMediaDevices = {
    getUserMedia: jest.fn().mockResolvedValue(fakeMedia),
  };

  Object.defineProperty(window.navigator, 'mediaDevices', {
    value: mockMediaDevices,
  });

  Object.defineProperty(HTMLVideoElement.prototype, 'videoHeight', {
    get: () => videoHeight,
  });

  Object.defineProperty(HTMLVideoElement.prototype, 'videoWidth', {
    get: () => videoWidth,
  });

  const mockCanvasRenderingContext = {
    drawImage: () => {},
    translate: () => {},
    scale: () => {},
  }

  jest.spyOn(HTMLCanvasElement.prototype, 'getContext')
    .mockImplementation(() => mockCanvasRenderingContext)

  jest.spyOn(HTMLCanvasElement.prototype, 'toDataURL')
    .mockImplementation(() => image)
}

And then as an example of a test we have for our component which uses this library, we have:

import React from 'react';
import { render, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

const dummyImage = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAQMAAAD+wSzIAAAABlBMVEX///+/v7+jQ3Y5AAAADklEQVQI12P4AIX8EAgALgAD/aNpbtEAAAAASUVORK5CYII';

beforeEach(() => {
  mockReactWebcamScreenshot({ image: dummyImage })
});

afterEach(() => {
  jest.restoreAllMocks();
});

describe('WebcamPhoto', () => {
  it('submits screenshot on Capture', async () => {
    const submitSpy = jest.fn();
    const { getByRole } = render(<WebcamPhoto onSubmit={submitSpy} />);

    let captureButton = getByRole('button', { name: 'Capture photo' });
    expect(captureButton).toBeInTheDocument();

    userEvent.click(captureButton);

    expect(submitSpy).toHaveBeenCalledTimes(1);
    expect(submitSpy).toHaveBeenCalledWith(dummyImage);
  });
});

I'd love some feedback on this approach, and would also be curious if it makes sense to include some kind of test helper like this within this library so if the internal implementation changes we could keep the test helper in sync with the code?

shiraze commented 1 year ago

Hi @brycesenz I'm no longer involved in the project where I first encountered this issue. @sorinpav is, though, and I'm not sure whether the test we used to have in Enzyme has now been moved to React Testing Library. With RTL, though, it's best to avoid mocking (that's the whole point, innit?!), but maybe there's no way out for this scenario.

brycesenz commented 1 year ago

@shiraze - I hear your point about trying to avoid mocking; I just don't know another approach that is going to work with JSDom. Hoping others might see this and chime in with something better 🤷‍♂️