Skip to content

Remove tests in ReactDOMComponent-test depending on internal API #11337

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

Merged
merged 7 commits into from
Nov 10, 2017
Merged

Remove tests in ReactDOMComponent-test depending on internal API #11337

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ae96f21
Remove inputValueTracking from ReactDOMComponent-test dependency
AudyOdi Oct 20, 2017
cb7b802
prettier
AudyOdi Oct 23, 2017
3e9bfe3
use node._valueTracker and add some test cases to make sure that valu…
AudyOdi Oct 26, 2017
d480ce7
using Object.getOwnPropertyDescriptor to get the tracked value
AudyOdi Oct 30, 2017
4614636
move getValueTracker to each test case and use its corresponding prot…
AudyOdi Oct 31, 2017
aa75dde
remove tests and move the value tracker definition before React is im…
AudyOdi Nov 9, 2017
2dadb8d
Delete these tests completely
gaearon Nov 10, 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
39 changes: 32 additions & 7 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('ReactDOMComponent', () => {
var ReactTestUtils;
var ReactDOM;
var ReactDOMServer;
var inputValueTracking;

function normalizeCodeLocInfo(str) {
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
Expand All @@ -26,8 +25,6 @@ describe('ReactDOMComponent', () => {
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
// TODO: can we express this test with only public API?
inputValueTracking = require('../client/inputValueTracking');
});

describe('updateDOM', () => {
Expand Down Expand Up @@ -1152,18 +1149,46 @@ describe('ReactDOMComponent', () => {
container,
);

var tracker = inputValueTracking._getTrackerFromNode(inst);
var getValueTracker = Object.getOwnPropertyDescriptor(
Copy link
Collaborator

@gaearon gaearon Nov 3, 2017

Choose a reason for hiding this comment

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

This should ideally be done before React is imported.

HTMLInputElement.prototype,
'value',
).get;

expect(tracker.getValue()).toEqual('foo');
expect(getValueTracker.call(inst)).toEqual('foo');

inst.defaultValue = 'new foo';
expect(getValueTracker.call(inst)).not.toEqual('new foo');
Copy link
Collaborator

@gaearon gaearon Nov 3, 2017

Choose a reason for hiding this comment

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

This check is redundant. You already check for the other value later.

expect(getValueTracker.call(inst)).toEqual('foo');

inst.value = 'bar';
expect(getValueTracker.call(inst)).toEqual('bar');

// even if the value changes to empty string, should not return to defaultValue
inst.value = '';
expect(getValueTracker.call(inst)).toEqual('');
});

it('should track textarea values', () => {
var container = document.createElement('div');
var inst = ReactDOM.render(<textarea defaultValue="foo" />, container);

var tracker = inputValueTracking._getTrackerFromNode(inst);
var getValueTracker = Object.getOwnPropertyDescriptor(
HTMLTextAreaElement.prototype,
'value',
).get;

expect(getValueTracker.call(inst)).toEqual('foo');

inst.defaultValue = 'new foo';
expect(getValueTracker.call(inst)).not.toEqual('new foo');
expect(getValueTracker.call(inst)).toEqual('foo');

inst.value = 'bar';
expect(getValueTracker.call(inst)).toEqual('bar');

expect(tracker.getValue()).toEqual('foo');
// even if the value changes to empty string, should not return to defaultValue
inst.value = '';
expect(getValueTracker.call(inst)).toEqual('');
});

it('should throw for children on void elements', () => {
Expand Down