-
-
Notifications
You must be signed in to change notification settings - Fork 506
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
Comments
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.
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 Indeed, expounding upon that might be good. Changing that from "common error" to "common issue" is probably a good idea too. |
Funny you mention In my testing, it didn't seem to matter whether or not the function was an arrow function defined directly inside the |
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 render() {
<div
ref={(dom) => {
console.log(dom);
this.ref = dom;
}}
/>
} Every time you trigger a rerender this function will be invoked twice: once with setRef = (dom) => {
console.log(dom);
this.ref = dom;
}
render() {
<div ref={this.setRef} />
} |
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. |
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 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. |
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, one call on initial render, double call with null and dom element on HMR. |
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 If you recreate this and see them having the same behavior, you have a problem in whatever environment you're setting this up in. |
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 This is worth clarifying and explaining better in the guide. I also belive that HMR worflow deserves a mention there. |
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 I believe the article does very poor job at answering that question. |
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.
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 |
On the Refs guide page, there is a note:
🔗The relevant source file
🔗The article
Screenshot of the piece in question
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?
The text was updated successfully, but these errors were encountered: