Are React v16 props frozen or sealed ?

[gusdaud-zz]

I've being trying to update my application to React 16 but I'm getting this error message in Chrome:

Uncaught TypeError: Cannot assign to read only property 'fontFamily' of object '#<Object>'

My pseudo code is currently like this:

const styles = {
  container: {
    fontSize: '14px',
    fontFamily: '"Helvetica Neue",
  },
};

class MyComp extends Component {
  render = () => {
    let theme = this.props.theme;
    styles.container.fontFamily = theme.font;

    return (<div style={styles}>Testing</div>);
  }
}

I've noticed that the error happens only the second time render is run.

Also if I test in the debugger Object.isFrozen(this.props) inside the render method I'm getting true as return.

Which means that this.props.theme.font is frozen by react and this state is being copied to my styles object. And on the second pass it cannot be replaced because its frozen.

This didn't happen in previous versions of React.

So, is React freezing properties now ?

[vamshi2000]

Can you please look in to this Syntax
fontFamily: '"Helvetica Neue" ---> i see one extra ' .

[travi]

i finally tracked down my issue of trying to upgrade from v16.0.0 to v16.1.0 to a similar issue. in my case, i have a unit test that is trying to stub renderToStaticMarkup on react-dom/server using sinon, but i see that the export from react-dom/cjs/react-dom-server.node.development is frozen as:

Object.freeze({
	renderToString: renderToString,
	renderToStaticMarkup: renderToStaticMarkup,
	renderToNodeStream: renderToNodeStream,
	renderToStaticNodeStream: renderToStaticNodeStream,
	version: version
});

which results in the following error being thrown in my test:

TypeError: Cannot redefine property: renderToStaticMarkup
    at Function.defineProperty (<anonymous>)
    at wrapMethod (node_modules/sinon/lib/sinon/util/core/wrap-method.js:92:16)
    at stub (node_modules/sinon/lib/sinon/stub.js:60:44)
    at Object.stub (node_modules/sinon/lib/sinon/collection.js:102:33)
    at Context.setup (test/unit/....)

is this frozen intentionally? if so, is there a suggested work around for testing?

[gusdaud-zz]

@vamshi2000 this is a typo while I was writing the pseudo code but the actual syntax is correct.

@travi I took a while to figure too and the deal is that it spreads! even strings are copied as reference and frozen so it prevents future modifications in the destination object.

Overall, inspite it may give performance gains I think freezing takes more than it adds.

React team, what’s your thoughts ?

[travi]

it appears to not be frozen in the source, so i assume that it gets added by the transpilation?

[gaearon]

I don't necessarily think the issue described by OP and the issue in #11520 (comment) are related.

Could each of you please create a minimal example reproducing it?

#11520 (comment) is definitely caused by Rollup transpilation. We can fix it by changing the server bundle entry points like ReactDOMServerNode and ReactDOMServerBrowser to have a default rather than named export (like in ReactDOM), and then do the .default check like we do in react-dom/index.js. This is not a great fix long term but it would work. Want to send a PR to fix this?

The issue in the original post is less clear to me. We'll need a reproducing example for that one. I'm actually not convinced it is a bug. React does freeze the props and style objects but that wouldn't freeze individual props (only the object containing them). That is, unless you're somehow passing them as a prop or style object to something. Need to see the whole code to say more.

[travi]

thanks for the quick response. i can try to get a repro together tonight and see if i follow your suggestion well enough to put a quick PR together too. i'm pretty sure i follow, but will look a little deeper once i have a little more time to confirm.

since you think these are unrelated to each other, would you rather i create a separate issue to simplify tracking each one?

[gaearon]

since you think these are unrelated to each other, would you rather i create a separate issue to simplify tracking each one?

Yes please.

[travi]

i split out #11526

[gusdaud-zz]

I couldn't reproduce the issue on a fiddle that I've created: https://jsfiddle.net/wro6hcrd/
so mine may have being caused by something else. I'll investigate it further.

Thanks so far,

[gaearon]

I think what's happening is that you're passing styles.container as a style to some DOM element.

<div style={styles.container} />

In this case of course it is not allowed to mutate it later:

styles.container.fontFamily = something;

You are not allowed to mutate objects you pass as styles to React because then the style will be different between re-renders. This is very hard to debug and was a common source of bugs before we started freezing styles.

Mutating styles was deprecated in React 15. If you run this sort of code, React 15 will warn you that you're mutating a style and it won't work in a future version of React. React 16 tightened this up with Object.freeze.

I hope this helps!

[gaearon]

Note that the above comment is about freezing style objects (new in 16 but mutating has been deprecated earlier). It is not about freezing props.

Props have been frozen since React 0.14 (two years ago). Freezing doesn't work recursively. Only the this.props objects get frozen, not the objects you put in props (again, with the exception of the objects that end up as styles). This is not a new behavior in 16. However, some people bumped into this because they converted their code to ES6 classes, and thus opted into strict mode.

In strict mode, JS engine throws on mutation of a frozen object (as opposed to silently discarding it). Normally, you opt into strict mode with 'use strict' at the top of the file, or by using Babel (which currently always emits it IIRC), or by using ES modules (per spec). But if you didn't do either of these, and then convert your code to use classes, you opt into strict mode for their methods, and this makes silent failures visible.