Preact: ref-callback life-cycle order issue

[mindplay-dk]

I'm trying to auto-focus and auto-select an <input type="text"> via the ref property, and was surprised to find that ref gets invoked prior to value getting populated on the same element.

Working example in React:

https://codesandbox.io/s/38k089rxkm

Same example in Preact:

https://codesandbox.io/s/v6406xj99l

Under Preact, calling el.select() has no effect, because the element has no value yet.

Now, I could just defer with setTimeout() or whatever, but this appears to be a potentially substantial inconsistency from React, so I thought I should report it.

Possibly related to #477 and #814.

[marvinhagemeister]

@mindplay-dk good catch!

Put some logging in there and noticed that react sometimes has to ref calls (ref(null) + ref(element)) but sometimes it is only called with the element. preact only ever calls ref once. In either scenario react makes sure that the ref is called after the changes have been applied to the dom. Seems like we don't do that.

[mindplay-dk]

@marvinhagemeister so it never calls ref(null)?

That would be bad, since (besides deviating from React behavior) that means you can't clean up expired DOM element references.

Perhaps that should be tracked as a separate issue though?

[marvinhagemeister]

@mindplay-dk That's not what I said We correctly call ref with null when the ref is updated. Just not on initial mounting. React isn't strict about it either. Sometimes it does call ref(null) on mount sometimes it doesn't.

[mindplay-dk]

@marvinhagemeister I mean, that just sounds like a bug in React? There's no reason to call ref(null) initially - it's already null, so that's just misleading, the null call should indicate a lost reference.

[marvinhagemeister]

yep, sounds like it. Although it very likely won't have any effect for react users.

[developit]

I believe the spec for ref() mentions that it should not be used in a way that relies on it being called only for mount and unmount. This was one reason why createRef() was added to the library (and one reason why we're looking at add it to Preact). Ref was never designed to be used as a general-purpose element mount/unmount hook.

In terms of Preact's implementation, my understanding is that ref() should only be used to obtain and release references to DOM nodes, and that it will get invoked at some arbitrary time after an element is created or added to the DOM, but before the mount lifecycle methods on its parent component are called.

[mindplay-dk]

In terms of Preact's implementation, my understanding is that ref() should only be used to obtain and release references to DOM nodes, and that it will get invoked at some arbitrary time after an element is created or added to the DOM

Reasoning about intended use etc., I like to refer to the documentation rather than the implementation.

From the relevant documentation page:

Refs provide a way to access DOM nodes or React elements created in the render method.

That's the headline, and that' a pretty broad description.

Under "when to use refs":

Managing focus, text selection, or media playback

Text selection wouldn't be possible, and that's the case/example where I ran into problems, so.

It also says:

ref updates happen before componentDidMount or componentDidUpdate lifecycle hooks

Since these life-cycle hooks happen after props are applied, it'd be reasonable to expect ref to happen not at some random point during the previous life-cycle step, but between complete steps of a life-cycle step - while it doesn't explicitly state that one way or the other, it would seem reasonable to assume it doesn't trigger events half-way through creating or updating and element, I think?

For the record, I also poked through the ReactDOM test-suite, but didn't find anything that tests for this, so I guess technically it's unspecified.

[developit]

@mindplay-dk all good points. I think we should be able to switch back to invoking refs as part of the element lifecycle rather than during property diffing.

[marvinhagemeister]

Through some debugging it looks like there are 2 issues at play. One is that we call the ref too early before the changes have been applied to the DOM and the second is that the input doesn't receive focus.

The first one will be fixed in the next major version. I just opened a PR (private branch) that invokes refs as part of the element lifecycle and not like regular props per @developit suggestion.

The focus issue is a more complex one. It seems like this is a known bug in Chrome. When input.focus() is called too soon, the event is somehow dropped. Looks like it is the same issue as described here: https://stackoverflow.com/questions/17384464

With that in mind the workaround would be:

function autoFocus(el) {
  if (el) {
    // Chrome bug: It will somehow drop the focus event if it fires too soon.
    // See https://stackoverflow.com/questions/17384464/
    setTimeout(() => el.focus(), 1);
  }
}

[mindplay-dk]

@marvinhagemeister is setTimeout with very small timeout value reliable? In my experience, this can randomly end up behaving slightly differently under load. If a zero timeout value doesn't work, there's a chance this will fail under certain conditions - perhaps try something like setTimeout() => setTimeout() => el.focus()) which looks a little odd, but should effectively end up waiting for two browser cycles, regardless of how long those take, e.g. regardless of load.

(disclaimer: it's quite possible I'm completely wrong about this, just based on my experience debugging issues that require waiting for some kind of internal browser process to cycle...)

[marvinhagemeister]

@mindplay-dk setTimeout(fn, 0) should work too. focus() just needs to be called sometime after the DOM node is created.

[mindplay-dk]

@marvinhagemeister if a timeout of 0 works just as well, I'd definitely prefer that, as a timeout of 1 might imply that it wouldn't (always) work with a timeout of 0.

[mindplay-dk]

@marvinhagemeister did you deliberately close this? is it fixed?

[marvinhagemeister]

@mindplay-dk Oh no that was done by accident. I just merged the new ref implementation in our private repo for the next major release and because I mentioned this issue it got closed here... Wasn't aware that GitHub allowed closing issues from other repos.

[gaearon]

Put some logging in there and noticed that react sometimes has to ref calls (ref(null) + ref(element)) but sometimes it is only called with the element.

React doesn't do this for initial mount. If you look at the call stack when you get null you'll see that it's CodeSandbox trying to clean up React trees before performing a hot reload:

Cleaning up at the top is expected to detach refs.

[marvinhagemeister]

@gaearon Ohh that's a good point💯 Didn't even cross my mind that hmr could be responsible for the ref call. Thanks for chiming in 👍👍

[gaearon]

Well, not HMR itself, just whatever CodeSandbox did on top of it.

[mindplay-dk]

@gaearon nice catch 👍

[rbiggs]

What I do for focus is access this.base inside componentDidMount:

componentDidMount() {
  this.base.querySelector('input').focus()
}

[developit]

Related: #1225

This was likely a regression caused by https://github.com/developit/preact/commit/cf3970dcd98d76dcadbf61f72139fa516d511aa6 and could potentially be fixed by reverting it.

[marvinhagemeister]

This is fixed in Preact x :tada: