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 unit tests for concurrent mode event dispatching #14415

Merged
merged 1 commit into from
Jan 23, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
186 changes: 186 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -491,5 +491,191 @@ describe('ReactDOMFiberAsync', () => {
expect(container.textContent).toEqual('1');
expect(returnValue).toBe(undefined);
});

it('ignores discrete events on a pending removed element', () => {
const disableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

let formSubmitted = false;

class Form extends React.Component {
state = {active: true};
disableForm = () => {
this.setState({active: false});
};
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>
);
}
}

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

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

// Dispatch a click event on the Disable-button.
let firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
disableButton.dispatchEvent(firstEvent);

// There should now be a pending update to disable the form.

// This should not have flushed yet since it's in concurrent mode.
let submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// In the meantime, we can dispatch a new client event on the submit button.
let secondEvent = document.createEvent('Event');
secondEvent.initEvent('click', true, true);
// This should force the pending update to flush which disables the submit button before the event is invoked.
submitButton.dispatchEvent(secondEvent);

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

expect(submitButtonRef.current).toBe(null);
});

it('ignores discrete events on a pending removed event listener', () => {
const disableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

let formSubmitted = false;

class Form extends React.Component {
state = {active: true};
disableForm = () => {
this.setState({active: false});
};
submitForm = () => {
formSubmitted = true; // This should not get invoked
};
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>{' '}
: null}
</div>
);
}
}

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

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

// Dispatch a click event on the Disable-button.
let firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
disableButton.dispatchEvent(firstEvent);

// There should now be a pending update to disable the form.

// This should not have flushed yet since it's in concurrent mode.
let submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// In the meantime, we can dispatch a new client event on the submit button.
let secondEvent = document.createEvent('Event');
secondEvent.initEvent('click', true, true);
// This should force the pending update to flush which disables the submit button before the event is invoked.
submitButton.dispatchEvent(secondEvent);

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

it('uses the newest discrete events on a pending changed event listener', () => {
const enableButtonRef = React.createRef();
const submitButtonRef = React.createRef();

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}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For removed event listeners, it is a bit easier since we have one before hand and in it we can check if we should just ignore it.

This case is trickier because we go from not having a listener to later having one. So to implement this we need to have a listener attached before we need 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.

Now that I think about it, we probably have a small bug here today since we lazily attach certain event listeners to the root the first time they're used. If a different discrete event happens, then we attach a new discrete event for the first time, then we'd see this bug today. I think the only fix is to add all of them to the document eagerly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you saying that there is a bug in the existing code today? If so, why isn't the test failing currently? Shouldn't we try to fix this bug in current ReactDOM, so we can have a regression test for Fire?

ref={submitButtonRef}>
Submit
</button>{' '}
: null}
</div>
);
}
}

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

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

// Dispatch a click event on the Enable-button.
let firstEvent = document.createEvent('Event');
firstEvent.initEvent('click', true, true);
enableButton.dispatchEvent(firstEvent);

// There should now be a pending update to enable the form.

// This should not have flushed yet since it's in concurrent mode.
let submitButton = submitButtonRef.current;
expect(submitButton.tagName).toBe('BUTTON');

// In the meantime, we can dispatch a new client event on the submit button.
let secondEvent = document.createEvent('Event');
secondEvent.initEvent('click', true, true);
// This should force the pending update to flush which enables the submit button before the event is invoked.
submitButton.dispatchEvent(secondEvent);

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