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.

Sign up for GitHub

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

Jump to bottom

Use only public API for ChangeEventPlugin-test.js #11333

Merged
merged 13 commits into from
Nov 24, 2017
Merged

Use only public API for ChangeEventPlugin-test.js #11333

Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
29253ec
Use only public API for ChangeEventPlugin-test.js
Ethan-Arrowood Oct 23, 2017
df01d2b
precommit commands complete
Ethan-Arrowood Oct 23, 2017
c7777f9
Removed comments
Ethan-Arrowood Oct 25, 2017
b6283f0
Merge branch 'master' into open-test-to-api
gaearon Oct 26, 2017
27ec33f
Merge branch 'master' into open-test-to-api
jquense Nov 6, 2017
bff4f3b
Improving event dispatchers
Ethan-Arrowood Nov 12, 2017
8be2a24
Merge remote-tracking branch 'upstream/master' into open-test-to-api
Ethan-Arrowood Nov 24, 2017
054c955
Updated tests
Ethan-Arrowood Nov 24, 2017
6238df3
Fixed for revisions
Ethan-Arrowood Nov 24, 2017
852e663
Prettified
Ethan-Arrowood Nov 24, 2017
7bb2124
Add more details and fixes to tests
gaearon Nov 24, 2017
0570084
Not internal anymore
gaearon Nov 24, 2017
d34a205
Remove unused code
gaearon Nov 24, 2017
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
223 changes: 115 additions & 108 deletions packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,212 +11,219 @@

var React = require('react');
var ReactDOM = require('react-dom');
var ReactTestUtils = require('react-dom/test-utils');
// TODO: can we express this test with only public API?
var ChangeEventPlugin = require('../ChangeEventPlugin').default;
var inputValueTracking = require('../../client/inputValueTracking');

function getTrackedValue(elem) {
var tracker = inputValueTracking._getTrackerFromNode(elem);
return tracker.getValue();
return elem.value;
}

function setTrackedValue(elem, value) {
var tracker = inputValueTracking._getTrackerFromNode(elem);
tracker.setValue(value);
}

function setUntrackedValue(elem, value) {
var tracker = inputValueTracking._getTrackerFromNode(elem);
var current = tracker.getValue();
var setUntrackedChecked = Object.getOwnPropertyDescriptor(
HTMLInputElement.prototype,
'checked',
).set;

if (elem.type === 'checkbox' || elem.type === 'radio') {
elem.checked = value;
} else {
elem.value = value;
}
tracker.setValue(current);
}
var setUntrackedValue = Object.getOwnPropertyDescriptor(
HTMLInputElement.prototype,
'value',
).set;

describe('ChangeEventPlugin', () => {
it('should fire change for checkbox input', () => {
var container;

beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);
});

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

fit('should fire change for checkbox input', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wrote "fit" in my example to force only a few tests to execute. It should not be used in committed code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it just be it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes.

var called = 0;

function cb(e) {
called = 1;
called++;
expect(e.type).toBe('change');
}

var input = ReactTestUtils.renderIntoDocument(
var node = ReactDOM.render(
<input type="checkbox" onChange={cb} />,
container,
);

setUntrackedValue(input, true);
ReactTestUtils.SimulateNative.click(input);

setUntrackedChecked.call(node, false);
node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));
Copy link
Collaborator

@gaearon gaearon Nov 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

We should leave a comment about jsdom click misbehaving. It's not obvious and doesn't happen in the browser (it's also possible jsdom has fixed this in new versions so we need to make it clear in case somebody later tries to update it, and the test will fail).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please elaborate on what a 'common' is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

node.dispatchEvent(new Event('click'));
expect(called).toBe(0);

Is this what you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, autocorrect. Meant "we should leave a comment". Like in the sample I wrote above.

expect(called).toBe(1);
});
expect(node.checked).toBe(true);

it('should catch setting the value programmatically', () => {
var input = ReactTestUtils.renderIntoDocument(
<input type="text" defaultValue="foo" />,
);

input.value = 'bar';
expect(getTrackedValue(input)).toBe('bar');
setUntrackedChecked.call(node, true);
node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));
expect(called).toBe(2);
expect(node.checked).toBe(false);
});

it('should not fire change when setting the value programmatically', () => {
fit('should not fire change setting the value programmatically', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

var input = ReactTestUtils.renderIntoDocument(
<input type="text" onChange={cb} defaultValue="foo" />,
var input = ReactDOM.render(
<input type="text" defaultValue="foo" onChange={cb} />,
container,
);

input.value = 'bar';
ReactTestUtils.SimulateNative.change(input);
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
expect(called).toBe(0);

setUntrackedValue(input, 'foo');
ReactTestUtils.SimulateNative.change(input);
setUntrackedValue.call(input, 'foo');
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));

expect(called).toBe(1);
expect(input.value).toBe('foo');

input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
expect(called).toBe(1);
});

it('should not fire change when setting checked programmatically', () => {
fit('should not fire change when setting checked programmatically', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

var input = ReactTestUtils.renderIntoDocument(
var input = ReactDOM.render(
<input type="checkbox" onChange={cb} defaultChecked={true} />,
container,
);

//set programmatically
input.checked = true;
ReactTestUtils.SimulateNative.click(input);

setUntrackedChecked.call(input, false);
input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));
expect(called).toBe(0);

input.checked = false;
setTrackedValue(input, undefined);
ReactTestUtils.SimulateNative.click(input);
setUntrackedChecked.call(input, true);
input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));

expect(called).toBe(1);
});

it('should unmount', () => {
var container = document.createElement('div');
fit('should catch setting the value programmatically', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand what this test is trying to test. What is the thinking behind it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should test that the tracked value updates when you set it from js. The test should normally set the value then trigger and event and assert that the event doesn't fire

Copy link
Collaborator

@gaearon gaearon Nov 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I meant that I don't understand what the new (rewritten) test is doing (since it's not testing setting the field at all).

var input = ReactDOM.render(
<input type="text" defaultValue="foo" />,
container,
);
setUntrackedValue.call(input, 'bar');
expect(getTrackedValue(input)).toBe('bar');
});

fit('should unmount', () => {
var input = ReactDOM.render(<input />, container);

ReactDOM.unmountComponentAtNode(container);
});

it('should only fire change for checked radio button once', () => {
fit('should only fire change for checked radio button once', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

var input = ReactTestUtils.renderIntoDocument(
var input = ReactDOM.render(
<input type="radio" onChange={cb} />,
container,
);
setUntrackedValue(input, true);
ReactTestUtils.SimulateNative.click(input);
ReactTestUtils.SimulateNative.click(input);

setUntrackedChecked.call(input, true);
input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true}));
expect(called).toBe(1);
});

it('should deduplicate input value change events', () => {
var input;
fit('should deduplicate input value change events', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

[
<input type="text" onChange={cb} />,
<input type="number" onChange={cb} />,
<input type="range" onChange={cb} />,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like it was important to do this for all types of inputs, and not just 'text'?

].forEach(function(element) {
called = 0;
input = ReactTestUtils.renderIntoDocument(element);

setUntrackedValue(input, '40');
ReactTestUtils.SimulateNative.change(input);
ReactTestUtils.SimulateNative.change(input);
expect(called).toBe(1);

called = 0;
input = ReactTestUtils.renderIntoDocument(element);
setUntrackedValue(input, '40');
ReactTestUtils.SimulateNative.input(input);
ReactTestUtils.SimulateNative.input(input);
expect(called).toBe(1);

called = 0;
input = ReactTestUtils.renderIntoDocument(element);
setUntrackedValue(input, '40');
ReactTestUtils.SimulateNative.input(input);
ReactTestUtils.SimulateNative.change(input);
expect(called).toBe(1);
});
var input = ReactDOM.render(<input type="text" onChange={cb} />, container);

setUntrackedValue.call(input, 'foo');

input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

expect(called).toBe(1);

setUntrackedValue.call(input, 'bar');

input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));

expect(called).toBe(2);

setUntrackedValue.call(input, 'foobar');

input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

expect(called).toBe(3);
});

it('should listen for both change and input events when supported', () => {
fit('should listen for both change and input events when supported', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

if (!ChangeEventPlugin._isInputEventSupported) {
return;
}

var input = ReactTestUtils.renderIntoDocument(
var input = ReactDOM.render(
<input type="range" onChange={cb} />,
container,
);
setUntrackedValue(input, 'bar');

ReactTestUtils.SimulateNative.input(input);

setUntrackedValue(input, 'foo');
setUntrackedValue.call(input, '0');
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));

ReactTestUtils.SimulateNative.change(input);
setUntrackedValue.call(input, '1');
input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

expect(called).toBe(2);
});

it('should only fire events when the value changes for range inputs', () => {
fit('should only fire events when the value changes for range inputs', () => {
var called = 0;

function cb(e) {
called += 1;
called++;
expect(e.type).toBe('change');
}

var input = ReactTestUtils.renderIntoDocument(
var input = ReactDOM.render(
<input type="range" onChange={cb} />,
container,
);
setUntrackedValue(input, '40');
ReactTestUtils.SimulateNative.input(input);
ReactTestUtils.SimulateNative.change(input);
setUntrackedValue.call(input, '40');
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

setUntrackedValue(input, 'foo');
setUntrackedValue.call(input, 'foo');
input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

ReactTestUtils.SimulateNative.input(input);
ReactTestUtils.SimulateNative.change(input);
expect(called).toBe(2);
});
});