Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback Refs note on double call is confusing #1001

Open
L1Q opened this issue Jun 14, 2023 · 10 comments
Open

Callback Refs note on double call is confusing #1001

L1Q opened this issue Jun 14, 2023 · 10 comments

Comments

@L1Q
Copy link

L1Q commented Jun 14, 2023

On the Refs guide page, there is a note:

If the ref callback is defined as an inline function it will be called twice. Once with null and then with the actual reference. This is a common error and the createRef API makes this a little easier by forcing user to check if ref.current is defined.

🔗The relevant source file
🔗The article

Screenshot of the piece in question

image

This note is useful in the sense that it's correct, in the first part that is:
If the ref callback is defined as an inline function it will be called twice. Once with `null` and then with the actual reference.
Indeed the ref function will be called twice, e.g. during a Vite's HMR update.

Where it gets crazy is the second half:
This is a common error
Very ambiguous here. On whose behalf is the error? What is the actual issue? Do you keep reintroducing the "error" in every preact release?

Now after some looking around I figured, perhaps there is a good reason it's called twice?
From what I found, it looks like the first call ever is always non-null, the second call will pass null, immediately followed by the third call, again with an actual element passed.

It looks like null is passed during unmount, and element is passed during mount.

Is this a correct assumption? Can this be clarified in the guide?

@rschristian
Copy link
Member

On whose behalf is the error? What is the actual issue? Do you keep reintroducing the "error" in every preact release?

It's an issue for the users, as it might be unexpected. It's not an "error" in Preact releases, but fundamentally how the API works.

Now after some looking around I figured, perhaps there is a good reason it's called twice?

If you define the function inline, every render will create a new instance of this function. Because of this, the function will be called once with null to clear it out then again with the actual DOM element to set the ref back up.

Indeed, expounding upon that might be good. Changing that from "common error" to "common issue" is probably a good idea too.

@L1Q
Copy link
Author

L1Q commented Jun 14, 2023

Funny you mention inline specifically. Perhaps my understanding of this term is different from yours.

In my testing, it didn't seem to matter whether or not the function was an arrow function defined directly inside the ref={} block. Defining it globally using a function keyword and then passing it to the ref prop produced the same behaviour.

@rschristian
Copy link
Member

Funny you mention inline specifically.

As the note says, this is only an issue for inline functions.

Here's a little example that you can test with. Simply replace the ref with ref=${this.setRef}.

render() {
    <div
        ref={(dom) => {
            console.log(dom);
            this.ref = dom;
        }}
    />
}

Every time you trigger a rerender this function will be invoked twice: once with dom being null, the second time with it being the div element. If this isn't an inline function, however, you'll get nothing logged to the console on rerenders, as the function isn't being recreated (and then reran) on rerenders.

setRef = (dom) => {
    console.log(dom);
    this.ref = dom;
}

render() {
    <div ref={this.setRef} />
}

@L1Q
Copy link
Author

L1Q commented Jun 15, 2023

Indeed, the distinction you make here is between a class method and an arrow function, not between an arrow function and a regular named function.

Perhaps the note on the guide page could be more clear about this as well.

@rschristian
Copy link
Member

rschristian commented Jun 15, 2023

Indeed, the distinction you make here is between a class method and an arrow function

No, that was incidental. The intended distinction here is between inline and non-inline functions. Arrow function or non-arrow, makes no difference whatsoever (besides accessing w/ this, but that can be worked around). It's only the inline functions that are an issue.

This will present the same problem, despite there being no arrow functions:

render() {
    <div
        ref={function (dom) {
            console.log(dom);
            ref = dom;
        }}
    />
}

...and this doesn't rely on class methods to avoid it:

let ref = null;
function setRef(dom) {
  console.log(dom);
  ref = dom;
}

class App extends Component {
  render() {
    <div ref={setRef} />
  }
}

Inline functions (functions defined within your returned JSX) are the problem here. Arrow functions, non-arrow functions, and class methods are all unrelated to the issue.

@L1Q
Copy link
Author

L1Q commented Jun 15, 2023

I have recreated your example with both

function setRef(dom) {
  console.log(dom);
  ref = dom;
}

and

        ref={function (dom) {
            console.log(dom);
            ref = dom;
        }}

both as a class component and as a function compoments,
and they all behave absolutely the same for me.

one call on initial render, double call with null and dom element on HMR.

@rschristian
Copy link
Member

HMR is a different thing entirely -- ignore it, it has no bearing on production behavior. It's not relevant here.

What the note refers to is rerenders and inline callback functions only. Not initial renders, not HMR, not arrow or class methods etc. Just inline callback refs.

Basically this example will log null in addition to the ref when you click on the button (triggering a rerender), while this one won't, as it's not inline.

If you recreate this and see them having the same behavior, you have a problem in whatever environment you're setting this up in.

@L1Q
Copy link
Author

L1Q commented Jun 15, 2023

Finally, I see what you mean. When testing non-HMR scenarios, the behaviour is exactly as you describe. pretty much only functions defined directly inside ref={} block would run twice, and only during rerenders, not initial renders.

This is worth clarifying and explaining better in the guide.

I also belive that HMR worflow deserves a mention there.

@L1Q
Copy link
Author

L1Q commented Jun 15, 2023

Now this is perhaps a little offtopic, but consider this piece of code as well:

import { useState } from 'preact/hooks'

const refFunc = (dom: HTMLElement | null) => {
  console.log(dom)
}

export function App() {
  const [score, set_score] = useState<number>(0)
  return (<>
      {score % 2 ? <div ref={refFunc} /> : <></>}
      <button onClick={() => set_score(score + 1)}>click me {score}</button>
    </>)
}

In this example, the refFunc is not called twice, but it is called with null every other time. It's one reason why I came to the guilde in the first place. I use TypeScript and could see the function signature. I wanted to understand how is it possible to ever have the ref function argument to be null.

I believe the article does very poor job at answering that question.

@rschristian
Copy link
Member

rschristian commented Jun 15, 2023

I also belive that HMR worflow deserves a mention there.

It's best not to rely on HMR specifics, IMO. How it interacts with your components should be an implementation detail, not something to actively document.

HMR can be touchy at the best of times, so when in doubt, refresh the page and retest.

I wanted to understand how is it possible to ever have the ref function argument to be null.

Keep in mind that refs are references to DOM elements. If the element is conditionally rendered, and might not exist on the page, the reference is null. What would you be referencing? When refFunc is ran, the div won't yet exist on the page. This is why you see alternating null and div elements, as it alternates between existing and not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants