Skip to content

Commit

Permalink
[act] remove obsolete container element (#16312)
Browse files Browse the repository at this point in the history
In a previous version of act(), we used a dummy dom element to flush effects. This doesn't need to exist anymore, and this PR removes it. The warning doesn't need to be there either (React will fire a wrong renderer act warning if needed).
  • Loading branch information
Sunil Pai authored Aug 9, 2019
1 parent 66a4742 commit b9faa3b
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 43 deletions.
28 changes: 1 addition & 27 deletions packages/react-dom/src/test-utils/ReactTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import type {Thenable} from 'react-reconciler/src/ReactFiberWorkLoop';

import React from 'react';
import ReactDOM from 'react-dom';
Expand All @@ -19,7 +18,6 @@ import {
import SyntheticEvent from 'events/SyntheticEvent';
import invariant from 'shared/invariant';
import lowPriorityWarning from 'shared/lowPriorityWarning';
import warningWithoutStack from 'shared/warningWithoutStack';
import {ELEMENT_NODE} from '../shared/HTMLNodeType';
import * as DOMTopLevelEventTypes from '../events/DOMTopLevelEventTypes';
import {PLUGIN_EVENT_SYSTEM} from 'events/EventSystemFlags';
Expand Down Expand Up @@ -153,12 +151,6 @@ function validateClassInstance(inst, methodName) {
);
}

// a plain dom element, lazily initialized, used by act() when flushing effects
let actContainerElement = null;

// a warning for when you try to use TestUtils.act in a non-browser environment
let didWarnAboutActInNodejs = false;

/**
* Utilities for making it easy to test React components.
*
Expand Down Expand Up @@ -395,25 +387,7 @@ const ReactTestUtils = {
Simulate: null,
SimulateNative: {},

act(callback: () => Thenable) {
if (actContainerElement === null) {
if (__DEV__) {
// warn if we're trying to use this in something like node (without jsdom)
if (didWarnAboutActInNodejs === false) {
didWarnAboutActInNodejs = true;
warningWithoutStack(
typeof document !== 'undefined' && document !== null,
'It looks like you called ReactTestUtils.act(...) in a non-browser environment. ' +
"If you're using TestRenderer for your tests, you should call " +
'ReactTestRenderer.act(...) instead of ReactTestUtils.act(...).',
);
}
}
// now make the stub element
actContainerElement = document.createElement('div');
}
return act(callback);
},
act,
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1022,20 +1022,4 @@ describe('ReactTestRenderer', () => {
expect(Scheduler).toFlushWithoutYielding();
ReactTestRenderer.create(<App />);
});

// we run this test here because we need a dom-less scope
it('warns and throws if you use TestUtils.act instead of TestRenderer.act in node', () => {
// we warn when you try to load 2 renderers in the same 'scope'
// so as suggested, we call resetModules() to carry on with the test
jest.resetModules();
const {act} = require('react-dom/test-utils');
expect(() => {
expect(() => act(() => {})).toThrow('document is not defined');
}).toWarnDev(
[
'It looks like you called ReactTestUtils.act(...) in a non-browser environment',
],
{withoutStack: 1},
);
});
});

0 comments on commit b9faa3b

Please sign in to comment.