facebook/react

Ordering of componentWillMount/Unmount in React 16

Closed this issue · 9 comments

Problem

It seems that the ordering of componentWillMount and componentWillUnmount is no longer guaranteed in React 16 due to support for async componentWillUnmount.

Previously, componentWillUnmount was always called on old components being removed before componentWillMount was called on new components.

Example

We have a Form component in which inputs "register" themselves with the parent Form in componentWillMount and deregister themselves in componentWillUnmount (this allows the Form to keep track of global state for all of the inputs). The following scenario will cause the Form to throw an error now that ordering is no longer guaranteed:

{ showTextInput ? <TextInput name="a" /> : <CheckboxInput name="a" /> }

In this scenario, it's possible that the Form will try to register an input with the same name before the previous input has been unmounted, which is not allowed.

Question

While I understand the reasoning behind this change, I'm wondering what the suggested solution is if our code previously relied on componentWillUnmount firing before componentWillMount?

The most obvious solution that comes to mind is moving the registerInput step from willMount into didMount. However, this means we'll have to deal with an unnecessary re-render upon mounting the input. Is this still the correct approach?

Can you explain why registering would cause a re-render?

I think you’re right this needs to be in didMount.

Thanks for the quick response!

Basically when an input is registered, it adds itself to the Form's state with default props, i.e..:

// pseudocode
formStateObject[input.name] = {
  isFocused: false,
  isTouched: false,
  validity: {},
  value: input.defaultValue,
  ... // etc
}

These are then propagated back down to the input via context.

If we move to didMount, that means that none of those props are set on initial render (since the component hasn't been registered yet). Once the input is registered, the change to the global Form state on context triggers a re-render. Refactoring to use didMount wouldn't be too difficult, just want to confirm that it is the recommended approach.

Yes, that's the recommended approach.

Once the input is registered, the change to the global Form state on context triggers a re-render.

This sounds unfortunate. Do you think it might make sense to just not initialize them at all, and only create those fields lazily on first change?

elado commented

@gaearon [jumping on this thread]

Do you think it might make sense to just not initialize them at all, and only create those fields lazily on first change

possibly. we are relying on props like <Input defaultValue=.. to register the input initially so it's rendered the first time with defaultValue. also default flags like isTouched/isPristine get some default values that are expected in the Input the first render.

I assume the fix would be to return null from render initially, register on didMount, then render would work fine with initial props.

Curious why the order is changed also. I would imagine that when a component is replaced with another, it will first unmount and then the other one will mount, as it used to be in React 15.

Thanks!

The order is changed intentionally. It is mentioned in the release notes as a breaking change.

The reason being that we want React to be able to start work on new trees asynchronously while old trees are still visible. We can only do this if we start working on a tree (and doing first render) before the old tree has unmounted.

For your particular case, maybe there’s some way to keep the state that is interdependent hoisted (but initialised lazily), but the state that is determined by the component could initially in the component itself (and “gathered” by the form when needed for submission). I don’t have a great answer to this. If you create a minimal example demonstrating the need for this please create a new issue specifically about this pattern (form-field communication). Maybe we can come up with some API that could make those cases more natural. But it’s hard to discuss without code we could run.

elado commented

@gaearon

Thanks again for answering! Bottom line is, we have a solution, but I set up a sandbox with the code and the differences:

React 16 - https://codesandbox.io/s/00jv33v22v
React 15 - https://codesandbox.io/s/24384jqr1r

This is a very raw sample of what we're doing (the actual code involves more HOCs/wrappers). Form works kinda like an HTML form meets Redux, where it knows about all the inputs and their values and holds everything in context.

The side effect of 16 order change is that the register of the new input happens before the unregister of the old input, which is not ideal for the flow, and then the old input unregisters itself, which actually removes the new input.

The solution we took is to move from willmount for registration to didmount, and render null if the context hasn't filled yet with the input state.

Let's keep this open because the discussion is interesting and there may be some better ways to do it. Personally I don't have much time to look into this now though.

I think the conceptual issue in your examples is that this code only works correctly when there's at most one <Input> with the same name prop in the tree at any given time. So if the "new" registration with the same name happens before the "old" registration is removed, the wrong one ends up being removed.

That tells me that the problem is with using the name as a registration key. What you really want is to register and unregister an individual input, not an individual name. One solution could be to generate a unique ID in each Input. When you register an input with the form, pass both its unique ID and name. Same when unregistering. Importantly, keep the data in the form component keyed by the ID rather than by name. This way, the order wouldn't matter — even if fields with the same name exists at the same time, each input would read the correct data. And then when you actually need the name (e.g. for form submission) you could enforce that only one such field should exist (or ignore any but the first, for example).

But why do we even have this weird overlapping time when the new component "will mount" but the old one hasn't unmounted yet? As I mentioned above it's unavoidable to unlock future React features. But this is also the reason we don't recommend using componentWillMount, and especially doing mutations there. It's just like mutations in a constructor (which is also not recommended). So what's the alternative? One thing you could do is to move the "registration" into componentDidMount (did). This way the order will be the one you expect. However, now the first render of a field would be before the data for it exists. One way to fix is it to teach the field itself to do something like let data = formFromContext[name] || myDefaultState. So on first render we wouldn't have the data from parent yet, but we'd read our own default data. And during registration, we'd pass our default data upwards, thus "registering" it in the form's state.

It seems to me like the second solution is more in line with how React works. Hope this helps.

tha same issues with the redux-form (https://redux-form.com)