React: update props in commitUpdate()

[dakom]

Given an instance which contains some object, attempts to update that object in commitUpdate() result in an error since the object is frozen

What is the correct approach to set the new props to an instance in commitUpdate()?

I see in the ART renderer for example, it sets the properties on the instance directly (e.g. https://github.com/facebook/react/blob/master/src/renderers/art/ReactARTFiberEntry.js#L152)

Is that the correct approach? Is there no ability to wrap these changing properties in a container object?

[dakom]

(heavily edited the original question since I had a poor choice of object naming in my code which led to false assumptions. Specifically I had an object named "props" and I thought that might have been something that gets especially frozen.)

Rather, now that I've fixed that, I see the question is about updating _any_ object on the instance.)

[gaearon]

attempts to update that object in commitUpdate() result in an error since the object is frozen

Which object? We don't freeze instances so it must be something else that you're updating that's frozen. We only freeze React elements and their props, but those aren't the ones you need to mutate.

I see the question is about updating any object on the instance

Still not sure I understand. But an instance is just any object you provide. You're free to do anything you like with it, including mutating it, adding or removing properties, using nested objects, etc.

[dakom]

@gaearon - sorry for the delay!

I took a closer look at it - and I get the error only in dev mode, in production mode it's fine

Specifically, given commitUpdate(instance, ...) in the reconciler, if I attempt to modify instance.foo.bar I get the error. This does not happen if modifying instance.foo (i.e. if foo itself is a scalar)

[gaearon]

React doesn't limit anything you do with your instances. What error are you getting? Message, stack, etc. Can you provide a reproducing example?

[dakom]

Hmmm I thought maybe if I rebuild the react-reconciler package and install it that might be a fix... but now I'm getting Noop reconciler is disabled o_O

[dakom]

Ah... seems I'm working against an old version and the API changed ;)

[gaearon]

Yeah. Put your stuff into mutation object.

[gaearon]

Or, rather, some of it. Flow types in ReactFiberReconciler should help :-)

[dakom]

got it working again and can see the error... not sure what's most helpful - to create a new repo with a minimal setup or give you some debug info?

Object.isFrozen(instance.props) is true in dev mode, false in production mode

(props just happens to be the name of some object on the instance)

[dakom]

Ahhh... my mistake... I was just assigning the props that came into the createInstance directly, without copying it into a new empty object. Of course those props were frozen - as they should be :)

[dakom]

For the sake of clarity in case anyone else hits this issue - everything @gaearon said above is correct, React does _not_ freeze anything on the instance itself, rather it freezes the copy of props that are passed to the instance (or at least it does so in dev mode). This makes perfect sense because from that perspective, props are by definition immutable. That doesn't prevent you from making your own unfrozen copy and storing it as some other mutable object on the instance

[gaearon]

Ah. That makes sense. Haven't thought about it. Yes, you shouldn't want to modify those.

[dakom]

btw you're totally right, I don't need to keep them around (yet at least, hehe), The only reason I had it was to compare old to new which I can just do in prepareUpdate or commitUpdate :)

On that note - I'm using prepareUpdate+payload+commitUpdate instead of just commitUpdate (e.g. always sending UPDATE_SIGNAL in prepareUpdate)

Are there some guidelines as to which approach is better?

[gaearon]

If you don't need to update in some case then you can optimize by skipping the signal.

[dakom]

Am I correct in thinking that prepareUpdate might be a good place to deal with async props as well?

For example, let's say there's a component that loads some data from the net and then, only once that data is loaded, does something useful with it in render(). The way I'd solve this with React would usually be to give that component local state and then setState() on load. Even if this is wrapped into a HOC for reuse across components, same idea and there's some local state in the pipeline.

With a custom renderer - I'm thinking this could instead be handled in prepareUpdate() by mapping newProps into an object such that the keys of unloaded data are omitted. When the data is ultimately loaded, they are included.

Although this won't itself trigger a new prepareUpdate() when load is finished, in my use case there's a single global state which is constantly updated anyway according to frame tick, so prepareUpdate will be continually called to check the loaded data.

In terms of implementation details - the mapping function could come from the instance which checks some sort of cache of loaded data it has access to. Or using lenses could use a generic function and the property list comes from the instance. Just thinking out loud.

Thoughts?

[gaearon]

I don't recommend implementing things like this in a custom renderer (nor do I expect it to work well). Custom renderers are meant strictly for deciding how to render things to platform primitives, not for data loading and other concerns that are typically solved on application level. Renderers are too low level for that because they're completely unaware of components.