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

Add tests for non-React discrete events flushing in a microtask #20772

Merged
merged 3 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 55 additions & 73 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,6 @@ describe('ReactDOMFiberAsync', () => {
});

describe('concurrent mode', () => {
beforeEach(() => {
jest.resetModules();

ReactDOM = require('react-dom');
Scheduler = require('scheduler');
});

// @gate experimental
it('does not perform deferred updates synchronously', () => {
const inputRef = React.createRef();
Expand Down Expand Up @@ -399,28 +392,26 @@ describe('ReactDOMFiberAsync', () => {

let formSubmitted = false;

class Form extends React.Component {
state = {active: true};
disableForm = () => {
this.setState({active: false});
};
submitForm = () => {
function Form() {
const [active, setActive] = React.useState(true);
function disableForm() {
setActive(false);
}
function submitForm() {
formSubmitted = true; // This should not get invoked
};
render() {
return (
<div>
<button onClick={this.disableForm} ref={disableButtonRef}>
Disable
</button>
{this.state.active ? (
<button onClick={this.submitForm} ref={submitButtonRef}>
Submit
</button>
) : null}
</div>
);
}
return (
<div>
<button onClick={disableForm} ref={disableButtonRef}>
Disable
</button>
{active ? (
<button onClick={submitForm} ref={submitButtonRef}>
Submit
</button>
) : null}
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
Expand Down Expand Up @@ -461,33 +452,29 @@ describe('ReactDOMFiberAsync', () => {

let formSubmitted = false;

class Form extends React.Component {
state = {active: true};
disableForm = () => {
this.setState({active: false});
};
submitForm = () => {
function Form() {
const [active, setActive] = React.useState(true);
function disableForm() {
setActive(false);
}
function submitForm() {
formSubmitted = true; // This should not get invoked
};
disabledSubmitForm = () => {
}
function disabledSubmitForm() {
// The form is disabled.
};
render() {
return (
<div>
<button onClick={this.disableForm} ref={disableButtonRef}>
Disable
</button>
<button
onClick={
this.state.active ? this.submitForm : this.disabledSubmitForm
}
ref={submitButtonRef}>
Submit
</button>
</div>
);
}
return (
<div>
<button onClick={disableForm} ref={disableButtonRef}>
Disable
</button>
<button
onClick={active ? submitForm : disabledSubmitForm}
ref={submitButtonRef}>
Submit
</button>
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
Expand Down Expand Up @@ -526,29 +513,24 @@ describe('ReactDOMFiberAsync', () => {

let formSubmitted = false;

class Form extends React.Component {
state = {active: false};
enableForm = () => {
this.setState({active: true});
};
submitForm = () => {
formSubmitted = true; // This should happen
};
render() {
return (
<div>
<button onClick={this.enableForm} ref={enableButtonRef}>
Enable
</button>
<button
onClick={this.state.active ? this.submitForm : null}
ref={submitButtonRef}>
Submit
</button>{' '}
: null}
</div>
);
function Form() {
const [active, setActive] = React.useState(false);
function enableForm() {
setActive(true);
}
function submitForm() {
formSubmitted = true; // This should not get invoked
}
return (
<div>
<button onClick={enableForm} ref={enableButtonRef}>
Enable
</button>
<button onClick={active ? submitForm : null} ref={submitButtonRef}>
Submit
</button>
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
Expand Down
228 changes: 228 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

let React;

let ReactDOM;
let Scheduler;

describe('ReactDOMNativeEventHeuristic-test', () => {
let container;

beforeEach(() => {
jest.resetModules();
container = document.createElement('div');
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');

document.body.appendChild(container);
});

afterEach(() => {
document.body.removeChild(container);
});

function dispatchAndSetCurrentEvent(el, event) {
try {
window.event = event;
el.dispatchEvent(event);
} finally {
window.event = undefined;
}
}

// @gate experimental
// @gate enableDiscreteEventMicroTasks && enableNativeEventPriorityInference
it('ignores discrete events on a pending removed element', async () => {
const disableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

function Form() {
const [active, setActive] = React.useState(true);

React.useLayoutEffect(() => {
disableButtonRef.current.onclick = disableForm;
});

function disableForm() {
setActive(false);
}

return (
<div>
<button ref={disableButtonRef}>Disable</button>
{active ? <button ref={submitButtonRef}>Submit</button> : null}
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
root.render(<Form />);
// Flush
Scheduler.unstable_flushAll();

const disableButton = disableButtonRef.current;
expect(disableButton.tagName).toBe('BUTTON');

// Dispatch a click event on the Disable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() =>
dispatchAndSetCurrentEvent(disableButton, firstEvent),
).toErrorDev(['An update to Form inside a test was not wrapped in act']);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using act would defeat the point of this test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add as inline comment?


// There should now be a pending update to disable the form.
// This should not have flushed yet since it's in concurrent mode.
const submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// Discrete events should be flushed in a microtask.
// Verify that the second button was removed.
await null;
expect(submitButtonRef.current).toBe(null);
// We'll assume that the browser won't let the user click it.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original test was more interesting here because it fired an event and verified that React ignores it. However, if we fire an event manually on a DOM node, and listen directly on that node with a native listener, it would still fire. So I don't know what we should be testing here. I think just testing that the DOM node was removed is enough because the rest is up to the browser.

});

// @gate experimental
// @gate enableDiscreteEventMicroTasks && enableNativeEventPriorityInference
it('ignores discrete events on a pending removed event listener', async () => {
const disableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

let formSubmitted = false;

function Form() {
const [active, setActive] = React.useState(true);

React.useLayoutEffect(() => {
disableButtonRef.current.onclick = disableForm;
submitButtonRef.current.onclick = active
? submitForm
: disabledSubmitForm;
});

function disableForm() {
setActive(false);
}

function submitForm() {
formSubmitted = true; // This should not get invoked
}

function disabledSubmitForm() {
// The form is disabled.
}

return (
<div>
<button ref={disableButtonRef}>Disable</button>
<button ref={submitButtonRef}>Submit</button>
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
root.render(<Form />);
// Flush
Scheduler.unstable_flushAll();

const disableButton = disableButtonRef.current;
expect(disableButton.tagName).toBe('BUTTON');

// Dispatch a click event on the Disable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() => {
dispatchAndSetCurrentEvent(disableButton, firstEvent);
}).toErrorDev(['An update to Form inside a test was not wrapped in act']);

// There should now be a pending update to disable the form.
// This should not have flushed yet since it's in concurrent mode.
const submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// Discrete events should be flushed in a microtask.
await null;

// Now let's dispatch an event on the submit button.
const secondEvent = document.createEvent('Event');
secondEvent.initEvent('click', true, true);
dispatchAndSetCurrentEvent(submitButton, secondEvent);

// Therefore the form should never have been submitted.
expect(formSubmitted).toBe(false);
});

// @gate experimental
// @gate enableDiscreteEventMicroTasks && enableNativeEventPriorityInference
it('uses the newest discrete events on a pending changed event listener', async () => {
const enableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

let formSubmitted = false;

function Form() {
const [active, setActive] = React.useState(false);

React.useLayoutEffect(() => {
enableButtonRef.current.onclick = enableForm;
submitButtonRef.current.onclick = active ? submitForm : null;
});

function enableForm() {
setActive(true);
}

function submitForm() {
formSubmitted = true; // This should not get invoked
}

return (
<div>
<button ref={enableButtonRef}>Enable</button>
<button ref={submitButtonRef}>Submit</button>
</div>
);
}

const root = ReactDOM.unstable_createRoot(container);
root.render(<Form />);
// Flush
Scheduler.unstable_flushAll();

const enableButton = enableButtonRef.current;
expect(enableButton.tagName).toBe('BUTTON');

// Dispatch a click event on the Enable-button.
const firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
expect(() => {
dispatchAndSetCurrentEvent(enableButton, firstEvent);
}).toErrorDev(['An update to Form inside a test was not wrapped in act']);

// There should now be a pending update to enable the form.
// This should not have flushed yet since it's in concurrent mode.
const submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// Discrete events should be flushed in a microtask.
await null;

// Now let's dispatch an event on the submit button.
const secondEvent = document.createEvent('Event');
secondEvent.initEvent('click', true, true);
dispatchAndSetCurrentEvent(submitButton, secondEvent);

// Therefore the form should have been submitted.
expect(formSubmitted).toBe(true);
});
});