React: Invalid "Mutating `style` is deprecated." warning in 0.14.0-rc1?

[moroshko]

I get the following warning in 0.14.0-rc1:

Warning: div was passed a style object that has previously been mutated. Mutating style is deprecated. Consider cloning it beforehand. Check the render of Preview. Previous style: {"color":"#747474","backgroundColor":"#009988","fontSize":null,"fontWeight":"300"}. Mutated style: {"color":"#747474","backgroundColor":"#009988","fontSize":null,"fontWeight":"300"}.

I'm confused why this warning appears because the "Previous style" and the "Mutated style" look exactly the same.

To reproduce the issue:

$ git clone [email protected]:moroshko/accessible-colors.git
$ cd accessible-colors
$ git checkout 1d8efda7145ff316b78e60cc0f0ebe113da11367
$ npm install
$ npm start

Then open http://localhost:3000/dist/index.html in a browser, and:

1. Type "1" in the font size field
2. Blur that field
   Now you should see the warning in the console.

Why this warning is displayed? How could I fix it?

[sophiebits]

Looks like your fontSize is getting set to NaN. If you have it as any other value then you won't get the warning.

We can probably have shallowEqual treat two NaNs as the same.

[moroshko]

Is it NaN or null? The warning message says null.

Yep, I think it makes sense for shallowEqual to consider two nulls (or NaNs) to be the same.

[zpao]

JSON.stringify({a: NaN}) === '{"a":null}'

So I'm willing to bet @spicyj did his debugging right :)

[moroshko]

@zpao All I'm saying is that if fontSize is getting NaN, I expect to see NaN in the warning message.

[zpao]

Sure, I'm saying the language doesn't make that easy for us to do. NaN doesn't have a representation in JSON. We _could_ sub in "NaN" when stringifying and trade one lie for another - maybe that's not as bad as null but it's still a lie since the value isn't a string either.

[moroshko]

@zpao I'd prefer the 'NaN' lie rather than the null lie. That would give me more clues to debug the potential issue.

[syranide]

We can probably have shallowEqual treat two NaNs as the same.

It should use Object.is for comparison I'd say.

[programnext]

Is this fix in 0.14.3? I got a similar issue with react 0.14.3.
Warning: div was passed a style object that has previously been mutated. Mutating style is deprecated. Consider cloning it beforehand. Check the render of Modal.
Previous style: {display: "block", width: "700px", height: {}, marginLeft: NaN, marginTop: -249}.
Mutated style: {display: "block", width: "700px", height: {}, marginLeft: NaN, marginTop: -249}

[cody]

@programnext
The warning is caused by marginLeft being NaN.

[thanzen]

I see. Thanks @cody. Is there anyway to workaround the issue since I have no access to marginLeft prop directly.

[jimfb]

@thanzen We could probably make an error message that is a bit more informative (https://github.com/facebook/react/issues/5773), but specifying a margin of NaN seems like it should always be an error/warning. Can you elaborate on what you mean by "I have no access to marginLeft prop directly"? If you mean you are using a third-party component that sets the property incorrectly, that component author should fix the issue.

[syranide]

@jimfb Tangential, perhaps we should explicitly warn whenever NaN is passed to a style, it's wrong and very obviously a user error (but not always easily spotted).

[jimfb]

@syranide https://github.com/facebook/react/issues/5773 :)

[thanzen]

@jimfb, yes, it is a third party component, and I will log the issue under that repo instead.

[essekia]

I don't have any null or NaN values and still get this error when updating style via Dispatcher -> Store - > changeListening. Any thoughts?

Warning:divwas passed a style object that has previously been mutated. Mutatingstyleis deprecated. Consider cloning it beforehand. Check therenderofnull. Previous style: {width: "500px", float: "left", background: "#fff", marginTop: "10px", marginBottom: "10px", marginLeft: "10px", marginRight: "10px", textAlign: "left"}. Mutated style: {width: "500px", float: "left", background: "#fff", marginTop: "10px", marginBottom: "10px", marginLeft: "10px", marginRight: "10px", textAlign: "left", fontSize: "15px"}.

[sophiebits]

@essekia You added fontSize: 15px. You need to create a new style object instead of changing the existing one.

[essekia]

Used jQuery's extend to clone the object and that solved it. Thanks @spicyj

[bradbumbalough]

I ran into this issue while using Object.assign. I just set the target to be an empty object so there's a new memory reference.

... style={Object.assign({}, styles.input, this.state.error ? {color: 'red'} : {}) ...

[pansy199211]

I don't have any NaN or null, and there are no new style added. I get the same warning as above. Anyone have idea about this? thx~

warning.js?0260:44Warning:figurewas passed a style object that has previously been mutated. Mutatingstyleis deprecated. Consider cloning it beforehand. Check therenderofImgFigure. Previous style: {left: 0, top: 0}. Mutated style: {left: 511, top: 272}.

[gaearon]

@webpansy

The NaN issue is a false positive of the warning, but in other cases the warning should fire correctly. So check whether you are mutating style that you’re declaring inside of render() method of ImgFigure, just like the warning says. You should not mutate style (or any other prop, really) after it has been returned from render(). It’s hard to say more because I haven’t seen the code, but you might want to ask this on StackOverflow with an example reproducing the problem.

[AoDev]

@gaearon

Hello. I am using observables with React. Mutating the store is exactly what is being done so that the views subscribe to these changes and re-render if needed. I am using mobx.
I feel like this warning is unnecessary in that case. Cloning the style object from the store just creates more boilerplate and unnecessary operations. The view is observing changes in the styles values and re-render automatically.

For the context: I am moving a character on the screen. Sometimes I reset its position.

These two examples mutate the styles in the store. But in one case, the warning will appear while in the other case it won't. This shows some incoherence.

This does not throw a warning:

// In an action
store.position.x = 100
store.position.y = 100

// In render
const style = {top: `${this.props.position.y}px`, left: `${this.props.position.x}px`}
<div style={style}/>

This throws a warning:

// in an action
store.style.top = '100px'
store.style.left = '100px'

// in render
<div style={store.style} />

Mutation occurred in both cases. Remember that I use observables, so it's valid and expected to simply change values instead of using immutables or "simulate" them by cloning / creating new objects.

What is your opinion about this use case?

PS: I wasn't sure about creating a new issue for this but it felt like a continuation to your last comment. I can create a new issue.

[rickhanlonii]

@AoDev the first does not throw a warning because of this line:

const style = { ... };

That's creating a new style object with a new memory reference, so you're actually not passing a mutated object. In the second you're passing store.style which is the same object with the same reference before and after mutation.

You can fix the second with:

<div style={Object.assign({}, store.style)} />

Be sure to do this with care though because of the performance implications (passing props like this to pure children will always force a render, removing the benefits of pure components).

[AoDev]

@rickhanlonii Thanks, I realised this later but forgot I had commented here.

[feluxe]

@rickhanlonii

Be sure to do this with care though because of the performance implications (passing props like this to pure children will always force a render, removing the benefits of pure components).

I use pure components heavily and this is starting to become a big problem for me.

As far as I can conclude, I have two options:

1) Create a new object e.g. <div style={Object.assign({}, store.style)} /> to get rid of the warning. This will leave me with unnecessary re-renderings of all affected components.
2) Create the style object outside the component to get rid of unnecessary re-renderings. This will leave me with the mutation warning.

Is there anything I can do about it?