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

Rendering Using 'muted' attribute in a video tag prints a warning about flushing updates to the console. #470

Closed
ericdcobb opened this issue Aug 29, 2019 · 11 comments

Comments

@ericdcobb
Copy link

  • react-testing-library version: 9.1.1
  • react version: 16.9.0
  • node version:
  • npm (or yarn) version:

Relevant code or config:

Code Sandbox: https://codesandbox.io/embed/agitated-snyder-mvvgi

Component:

import React from "react";

const App = () => {
  return (
    <div className="App">
      <h1>Hello Scout</h1>
      <h2>Start editing to see some magic happen!</h2>
      <video muted />
    </div>
  );
};

export default App;

Test:

import React from "react";
import { render, wait } from "@testing-library/react";
import App from "./App";

describe("App", () => {
  it("renders", async () => {
    const { container } = render(<App />);
    await wait(() => expect(container.textContent).toContain("Scout"));
  });
});

What you did:

I noticed a waring in my logs when rendering a video element with the muted attribute:

Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.
    in video (at App.js:15)
    in div (at App.js:12)
    in App (at App.test.js:7)
@kentcdodds
Copy link
Member

Verified. I'm not sure what this could be! Would you mind digging a little deeper and see if you can figure out what's causing this warning and whether there's anything we can do within React Testing Library? Perhaps you could ask on https://spectrum.chat/testing-library/help-react

@ericdcobb
Copy link
Author

@kentcdodds Of course, I'd love to take a look around and see what I can see, though it might not be until after the long weekend here in the US that I am able to dive in.

@chriszwickerocteris
Copy link

I've run into the same issue. The problem is that setting the muted property on a HTMLMediaElement leads to an event being fired, which in turn leads to flushDiscreteUpdatesIfNeeded being called.

At first, I thought we should be setting <videoRef>.defaultMuted = true instead, but while that sets the muted attribute on the video element in the DOM, it doesn't actually mute sound. On the other hand, setting muted in react doesn't set the muted attribute in the DOM.

Unfortunately, I haven't found a simple fix (and there are other issues open about this, e.g. this). In case you're interested, my workaround is to use my own wrapper for the render function:

const renderIgnoringUnstableFlushDiscreteUpdates = (component: React.ReactElement) => {
  // tslint:disable: no-console
  const originalError = console.error;
  const error = jest.fn();
  console.error = error;
  const result = render(component);
  expect(error).toHaveBeenCalledTimes(1);
  expect(error).toHaveBeenCalledWith("Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.%s", expect.any(String));
  console.error = originalError;
  // tslint:enable: no-console
  return result;
};

@kentcdodds
Copy link
Member

Interesting. I think this is out of scope for React Testing Library as it appears to be something with React specifically. If anyone can come up with something we can do to improve things, then please feel free to comment further, but from what I can tell, there's not much we can do.

jeremija added a commit to peer-calls/peer-calls that referenced this issue Nov 16, 2019
Setting muted to video results in warning:

Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React
is already rendering

This seems to be a bug in React:

testing-library/react-testing-library#470
@bfintal
Copy link

bfintal commented Dec 25, 2019

@chriszwickerocteris thanks! Here's the same code in eslint:

const renderIgnoringUnstableFlushDiscreteUpdates = component => {
	/* eslint-disable no-console */
	const originalError = console.error
	const error = jest.fn()
	console.error = error
	const result = render( component )
	expect( error ).toHaveBeenCalledTimes( 1 )
	expect( error ).toHaveBeenCalledWith( 'Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.%s', expect.any( String ) )
	console.error = originalError
	/* eslint-enable no-console */
	return result
}

@asherccohen
Copy link

Just to confirm that this is a react issue with muted event on videos.

facebook/react#10389

@nirajsingh002
Copy link

nirajsingh002 commented Jul 11, 2020

I was facing this warning in react-testing-library. I had been looking for the solution for around 4-5 hours. This works for me.
I would like to add steps in the solution because I was facing how to wrap the render component.

Step 1: add this function at the top of the test file.
const renderIgnoringUnstableFlushDiscreteUpdates = (component) => {
// tslint:disable: no-console
const originalError = console.error;
const error = jest.fn();
console.error = error;
const result = render(component);
expect(error).toHaveBeenCalledTimes(1);
expect(error).toHaveBeenCalledWith(
'Warning: unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.%s',
expect.any(String),
);
console.error = originalError;
// tslint:enable: no-console
return result;
};

Step 2: Wrap the render component from the step 1 function:

EX: renderIgnoringUnstableFlushDiscreteUpdates(<ComponentName {...props} />);

Step 3: Add this code in setupTest.js file for run video correctly.

window.HTMLMediaElement.prototype.load = () => {
/* do nothing /
};
window.HTMLMediaElement.prototype.play = () => {
/
do nothing /
};
window.HTMLMediaElement.prototype.pause = () => {
/
do nothing /
};
window.HTMLMediaElement.prototype.addTextTrack = () => {
/
do nothing */
};

@rudnitskih
Copy link

rudnitskih commented Oct 17, 2020

For me the following snippet in jest setup is helped:

Object.defineProperty(HTMLMediaElement.prototype, 'muted', {
  set: () => {},
});

My video element usage is:

 <video src={src} autoPlay={true} loop={true} muted={true} playsInline={true} />

@antoniopacheco
Copy link

antoniopacheco commented Oct 23, 2020

this should be fixed along with:
facebook/react#20087

@mdzialo00
Copy link

For me the following snippet in jest setup is helped:

Object.defineProperty(HTMLMediaElement.prototype, 'muted', {
  set: () => {},
});

My video element usage is:

 <video src={src} autoPlay={true} loop={true} muted={true} playsInline={true} />

I love you my boi, it helped!

Just paste this code guys into setupTest.js and it's not showing those weird erros! ❤

@michaelbayday
Copy link

used above^

zwliew added a commit to opengovsg/postmangovsg that referenced this issue Apr 1, 2021
This is mainly an issue in test environments. React treats 'muted' as a
property and sets it after the test environment has been torn down,
resulting in console warnings [1].

Fix this by mocking the setter property to do nothing. This doesn't pose
any issue in test environments.

[1]: testing-library/react-testing-library#470
zwliew added a commit to opengovsg/postmangovsg that referenced this issue Apr 12, 2021
This is mainly an issue in test environments. React treats 'muted' as a
property and sets it after the test environment has been torn down,
resulting in console warnings [1].

Fix this by mocking the setter property to do nothing. This doesn't pose
any issue in test environments.

[1]: testing-library/react-testing-library#470
zwliew added a commit to opengovsg/postmangovsg that referenced this issue Apr 15, 2021
* feat(npm): add npm script for testing frontend

* chore(npm): install frontend test dependencies

The following dependencies were installed:
- msw to mock backend API responses
- @testing-library/react for idiomatic react testing APIs
- @testing-library/jest-dom for idiomatic DOM assertion APIs
- @testing-library/user-event for idiomatic DOM user event APIs
- jest-canvas-mock to mock Canvas for Lottie as jsdom doesn't implement it
- @peculiar/webcrypto as jsdom doesn't implement SubtleCrypto

Also install the necessary TypeScript definitions.

* ga: allow init options to be passed in

The purpose is to pass in the testMode option during tests.

Note: I've also shifted the declaration of initialChecks() into the useEffect()
call where it is used. This is mainly to silence the React Hooks ESLint
warnings on missing dependencies in the useEffect() dependency list.

* feat(modal): allow initial content and no children

This allows us to specify the initial modal content when setting up test
environments.

* fix(config): Disregard REACT_APP env vars for tests

We don't care about these environment variables in tests.

Also, setting the axios baseURL for tests will result in flakiness in
tests that involve API responses.

* feat(test): set up initial test fixtures

* fix(HTMLMediaElement): mock setter for 'muted' attributes

This is mainly an issue in test environments. React treats 'muted' as a
property and sets it after the test environment has been torn down,
resulting in console warnings [1].

Fix this by mocking the setter property to do nothing. This doesn't pose
any issue in test environments.

[1]: testing-library/react-testing-library#470

* chore(travis): run the frontend tests

* chore(amplify): run the frontend tests

Make sure to set CI=true explicitly as Amplify doesn't set it by
default. CI=true will run all the tests exactly once and exit.

Also compile translations before running the tests.

* chore(app): move global context providers to index.tsx

We want to render App.tsx in a test environment with separate providers.
Hence, move the providers to index.tsx.

Also, move the `import 'locales'` statement to the top of index.tsx as
well.

* feat(test-utils): allow passing in router options

This allows us to specify initial history locations during tests.

Since we're making MemoryRouter the topmost provider, adjust index.tsx
to match.

* test(app): add an initial render test for App.tsx

Just a simple render test to ensure that the test infrastructure works fine.

* feat(setupTests): log an error on unhandled API requests

This allows us to more easily debug missing endpoints and fix them.

* chore(app): remove unnecessary mocked API endpoints

I'll look into how to best mock API endpoints across tests later on.
For now, only declare the endpoints necessary for App.test.tsx.

* fix(amplify): fix test configuration

* fix(travis): run frontend tests only after building

This makes more sense as compilation errors are more important than test
errors. This also matches the Amplify build configuration.

* fix(setupTest): reset msw server handlers after resetting timers

There may be pending timers that depend on API endpoints. Run all timers
to completion before resetting the API endpoints.

* chore(test): clean up test setup code and imports

* fix(test): properly redeclare the type of `global`

* fix(app): delete render test for App.tsx

For some reason, this test on its own is failing on Travis (but succeeds
on Amplify!) and I haven't been able to find the root cause.

However, adding more tests seem to fix the failure on Travis.
Hence, I'll just batch this test up along with the other
campaign creation integration tests.

* chore(test): pass the test suite if there are no tests

This makes sense as the "neutral element", and allows the test suite to
pass currently when we have zero tests.

* Revert "fix(app): delete render test for App.tsx"

This reverts commit 979b23e.

* refactor(ga): inject options from config.ts

* Remove the `gaOptions` parameter
* Define all gaOptions in config.ts, with props conditionally defined
  based on the current environment (test, dev, prod, etc.)

* fix(ga): define a dummy tracking ID for tests

This is to silence the console warning regarding a missing tracking ID.

* fix(setupTest): fake timers on a per-test basis

* refactor(app.test): rename the render test for clarity

* feat(app.test): add checks for the footer; improve docs

* chore(test): make /stats a common API mock

This API mock should be generic enough for all future tests.

* feat(test): convert /auth/userinfo to a common API mock

* Move /auth/userinfo to test-utils.tsx
* Allow custom user IDs in future tests

* chore(test-utils): refactor API mocks to more digestible chunks

* Split the API mocks by topic (stats, auth, etc.)

* feat(test-utils): return the state in mockCommonApis()

API mocks in individual tests can now manipulate the state based on the
needs of the test.

* refactor: shift tests around according to standardized structure

We've decided upon a standardized file structure for tests across
frontend and backend.

Example:

- src/
  - core/
    - routes/
      - auth.routes.ts
      - tests/ <-- `tests/` in the same directory as files to be tested
        - auth.routes.test.ts <-- tests in `tests/` directory
    - services/
      - phone-number.service.ts
      - tests/
        - phone-number.service.test.ts

* refactor: use import maps to import test-utils

* chore: add some inline comments for documentation

This might help anyone writing tests in the future to get started more
easily.

Co-authored-by: Lam Kee Wei <[email protected]>
miazima pushed a commit to opengovsg/postmangovsg that referenced this issue Apr 26, 2021
* feat(npm): add npm script for testing frontend

* chore(npm): install frontend test dependencies

The following dependencies were installed:
- msw to mock backend API responses
- @testing-library/react for idiomatic react testing APIs
- @testing-library/jest-dom for idiomatic DOM assertion APIs
- @testing-library/user-event for idiomatic DOM user event APIs
- jest-canvas-mock to mock Canvas for Lottie as jsdom doesn't implement it
- @peculiar/webcrypto as jsdom doesn't implement SubtleCrypto

Also install the necessary TypeScript definitions.

* ga: allow init options to be passed in

The purpose is to pass in the testMode option during tests.

Note: I've also shifted the declaration of initialChecks() into the useEffect()
call where it is used. This is mainly to silence the React Hooks ESLint
warnings on missing dependencies in the useEffect() dependency list.

* feat(modal): allow initial content and no children

This allows us to specify the initial modal content when setting up test
environments.

* fix(config): Disregard REACT_APP env vars for tests

We don't care about these environment variables in tests.

Also, setting the axios baseURL for tests will result in flakiness in
tests that involve API responses.

* feat(test): set up initial test fixtures

* fix(HTMLMediaElement): mock setter for 'muted' attributes

This is mainly an issue in test environments. React treats 'muted' as a
property and sets it after the test environment has been torn down,
resulting in console warnings [1].

Fix this by mocking the setter property to do nothing. This doesn't pose
any issue in test environments.

[1]: testing-library/react-testing-library#470

* chore(travis): run the frontend tests

* chore(amplify): run the frontend tests

Make sure to set CI=true explicitly as Amplify doesn't set it by
default. CI=true will run all the tests exactly once and exit.

Also compile translations before running the tests.

* chore(app): move global context providers to index.tsx

We want to render App.tsx in a test environment with separate providers.
Hence, move the providers to index.tsx.

Also, move the `import 'locales'` statement to the top of index.tsx as
well.

* feat(test-utils): allow passing in router options

This allows us to specify initial history locations during tests.

Since we're making MemoryRouter the topmost provider, adjust index.tsx
to match.

* test(app): add an initial render test for App.tsx

Just a simple render test to ensure that the test infrastructure works fine.

* feat(setupTests): log an error on unhandled API requests

This allows us to more easily debug missing endpoints and fix them.

* chore(app): remove unnecessary mocked API endpoints

I'll look into how to best mock API endpoints across tests later on.
For now, only declare the endpoints necessary for App.test.tsx.

* fix(amplify): fix test configuration

* fix(travis): run frontend tests only after building

This makes more sense as compilation errors are more important than test
errors. This also matches the Amplify build configuration.

* fix(setupTest): reset msw server handlers after resetting timers

There may be pending timers that depend on API endpoints. Run all timers
to completion before resetting the API endpoints.

* chore(test): clean up test setup code and imports

* fix(test): properly redeclare the type of `global`

* fix(app): delete render test for App.tsx

For some reason, this test on its own is failing on Travis (but succeeds
on Amplify!) and I haven't been able to find the root cause.

However, adding more tests seem to fix the failure on Travis.
Hence, I'll just batch this test up along with the other
campaign creation integration tests.

* chore(test): pass the test suite if there are no tests

This makes sense as the "neutral element", and allows the test suite to
pass currently when we have zero tests.

* Revert "fix(app): delete render test for App.tsx"

This reverts commit 979b23e.

* refactor(ga): inject options from config.ts

* Remove the `gaOptions` parameter
* Define all gaOptions in config.ts, with props conditionally defined
  based on the current environment (test, dev, prod, etc.)

* fix(ga): define a dummy tracking ID for tests

This is to silence the console warning regarding a missing tracking ID.

* fix(setupTest): fake timers on a per-test basis

* refactor(app.test): rename the render test for clarity

* feat(app.test): add checks for the footer; improve docs

* chore(test): make /stats a common API mock

This API mock should be generic enough for all future tests.

* feat(test): convert /auth/userinfo to a common API mock

* Move /auth/userinfo to test-utils.tsx
* Allow custom user IDs in future tests

* chore(test-utils): refactor API mocks to more digestible chunks

* Split the API mocks by topic (stats, auth, etc.)

* feat(test-utils): return the state in mockCommonApis()

API mocks in individual tests can now manipulate the state based on the
needs of the test.

* refactor: shift tests around according to standardized structure

We've decided upon a standardized file structure for tests across
frontend and backend.

Example:

- src/
  - core/
    - routes/
      - auth.routes.ts
      - tests/ <-- `tests/` in the same directory as files to be tested
        - auth.routes.test.ts <-- tests in `tests/` directory
    - services/
      - phone-number.service.ts
      - tests/
        - phone-number.service.test.ts

* refactor: use import maps to import test-utils

* feat(dashboard): write initial email campaign test

* feat(dashboard): add initial SMS campaign workflow test

Also refactor common API endpoints shared with the Email campaign test
into a separate array.

* feat(dashboard): test the flow for actually creating a new campaign

* feat(dashboard): add test for telegram campaign creation workflow

Also add assertions for testing the credential dropdown for SMS
campaigns.

* fix(email): select the correct channel button when creating email campaign

Previously, we were selecting the SMS channel button, not the email
button.

* feat(dashboard): colocate APIs; implement initial campaign management

* feat(dashboard): add happy path test for protected email campaign

* chore(test): rename dashboard integration tests for clarity

* fix(dashboard): fake timers for protected email integration test

* refactor: use the common mock APIs

* refactor: move /settings API mocks to the common APIs directory

* refactor: move the rest of the campaign APIs to the common directory

* refactor: move tests around according to standardized structure

* feat: support custom initial campaigns in the context provider

This allows tests to construct a custom initial campaign based on the
requirements of the test.

* refactor: use import maps to import test-utils

* fix(unsubscribe): use react-router to determine search params

In test environments, we use MemoryRouter instead of BrowserRouter for
routing. Since MemoryRouter does not manipulate `window.location`, we
cannot use `window.location` to determine the search parameters.

Instead, use the `location` object provided by react-router with the
`useLocation()` hook.

* feat: add a simple API mock for unsubscring from campaigns

* feat: add a test for successfully unsubscribing from a campaign

* fix: use fake timers for dashboard tests

* feat: mock protected message APIs

* feat: add unit test for successfully decrypting a protected message

* chore: correct a typo and add a setup comment

* feat(unsubscribe): add a test for when the user chooses to stay

* fix: add missing props to the mock API responses

1. Rename `templates` to `{email|sms|telegram}_templates` to match the
   actual API response
2. Convert `params` to an array to match the actual API
3. Add `has_credential` to match the actual API
4. Add a LOGGED job to the `job_queue` once a campaign is sent to better
   mimic the API

* fix: fix linting error due to missing `has_credential`

* refactor: split  Dashboard integration tests to separate files

Co-authored-by: Lam Kee Wei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants