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

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

nhunzaker
Copy link
Contributor

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

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;
```
@nhunzaker nhunzaker changed the title Inputs should not mutate value on type conversion Inputs should not mutate value on type conversion (when they stringify to the same thing) May 29, 2017
Copy link
Contributor

@aweary aweary left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@gaearon
Copy link
Collaborator

gaearon commented May 29, 2017

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

@gaearon
Copy link
Collaborator

gaearon commented May 30, 2017

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

@flarnie flarnie mentioned this pull request May 30, 2017
49 tasks
@flarnie
Copy link
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
flarnie pushed a commit that referenced this pull request May 31, 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:

```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 pushed a commit to flarnie/react that referenced this pull request Jun 7, 2017
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;
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants