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

Inputs should not mutate value on type conversion (when they stringify to the same thing) #9806

Merged
merged 1 commit into from May 30, 2017

Conversation

Projects
None yet
5 participants
@nhunzaker
Collaborator

nhunzaker commented May 29, 2017

This is a follow-up on #9584 (comment). There is no need to assign the value property of an input if the value property of the React component changes types, but stringifies to the same value. For example:

DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)

In this case, the assignment to input.value will always be cast to the string "true". There is no need to perform this assignment. Particularly when we already cast the value to a string later:

// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;

Testing

I've put up a build of the DOM fixtures for this branch here:

http://nh-input-type-conversion.surge.sh

Inputs should not mutate value on type conversion
This is a follow-up on
#9584 (comment). There
is no need to assign the value property of an input if the value
property of the React component changes types, but stringifies to the
same value. For example:

```javascript
DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)
```

In this case, the assignment to `input.value` will always be
cast to the string "true". There is no need to perform this
assignment. Particularly when we already cast the value to a string
later:

```javascript
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
```

@nhunzaker nhunzaker changed the title from Inputs should not mutate value on type conversion to Inputs should not mutate value on type conversion (when they stringify to the same thing) May 29, 2017

@aweary

aweary approved these changes May 29, 2017

Looks good to me 👍

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon May 29, 2017

Member

cc @flarnie We'll want to get this into 15.6, right?

Member

gaearon commented May 29, 2017

cc @flarnie We'll want to get this into 15.6, right?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon May 30, 2017

Member

@aweary If it looks good please merge 😉 Otherwise it might hang here for a long time.

Member

gaearon commented May 30, 2017

@aweary If it looks good please merge 😉 Otherwise it might hang here for a long time.

@flarnie flarnie referenced this pull request May 30, 2017

Closed

React 15.6 Umbrella #9398

41 of 49 tasks complete
@flarnie

This comment has been minimized.

Show comment
Hide comment
@flarnie

flarnie May 30, 2017

Contributor

I second what @gaearon said - @aweary or @nhunzaker feel free to merge and then I'm happy to cherry-pick this to 15.6-dev.

Contributor

flarnie commented May 30, 2017

I second what @gaearon said - @aweary or @nhunzaker feel free to merge and then I'm happy to cherry-pick this to 15.6-dev.

@aweary aweary merged commit 74044e0 into facebook:master May 30, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

flarnie added a commit that referenced this pull request May 31, 2017

Inputs should not mutate value on type conversion (#9806)
This is a follow-up on
#9584 (comment). There
is no need to assign the value property of an input if the value
property of the React component changes types, but stringifies to the
same value. For example:

```javascript
DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)
```

In this case, the assignment to `input.value` will always be
cast to the string "true". There is no need to perform this
assignment. Particularly when we already cast the value to a string
later:

```javascript
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
```

flarnie added a commit to flarnie/react that referenced this pull request Jun 7, 2017

Inputs should not mutate value on type conversion (#9806)
This is a follow-up on
facebook#9584 (comment). There
is no need to assign the value property of an input if the value
property of the React component changes types, but stringifies to the
same value. For example:

```javascript
DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)
```

In this case, the assignment to `input.value` will always be
cast to the string "true". There is no need to perform this
assignment. Particularly when we already cast the value to a string
later:

```javascript
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
```

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

Update dependency react to v16 #29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment