-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
in ref callback, ref is someitimes not part of the DOM yet #477
Comments
Hi there - preact doesn't guarantee refs are called after parent elements are mounted. Instead (and in general in React codebase), I recommend instead using the |
Hi thanks, yeah that is what I ended up doing. I couldn't find the documentation in react on how exactly that was supposed to work. Glad to know that's expected behavior. |
Great! Think we should close out this issue then? |
@developit Just running into an issue which is related to ref callback I think when trying out blueprintjs EditableText component with Preact. I need to investigate some more, but can you clarify whether Preact follows the same callback pattern as React here i.e.
Asking here as may be related to this issue. Sorry if you answered above, I wasn't totally clear on the answer. |
Currently Preact does not defer |
Great thanks for clarifying! I've not come across any issues around this before either, but for reference here's one example of where it seems breaking. https://github.com/palantir/blueprint/blob/master/packages/core/src/components/editable-text/editableText.tsx#L123 With Preact focus is not given to the input, I assume because it's not attached. Also with Preact, that ref callback seems to be getting called every time I type a char in the input, I've not had time to debug that one yet. |
This stumped me for a while too. Below is an example of some code where Preact requires I refactor it to ensure the event listeners are correctly added.
|
I'm having the same issue. I was assuming the ref callback to be triggered after mount/unmount. I cannot fix my problem easily using
That is, clonedChildren may not be mounted at the same time the parent component is mounted depending on SomeHoC... |
Hmm - @landabaso that doesn't seem any different from React's behavior. |
Yes, the ref is deferred. That works Ok. But when the callback is triggered, I need to access the I can access |
It's something I'm open to, just haven't had time to explore. Technically, refs should not be used to pierce component boundaries - that is what's causing the issue here - so the number of times this has come up is only twice. Happy to explore options though, just it's not something I've ever run into myself. I rarely use refs. |
I admit it's a strange situation. |
ahh yeah. the other case I saw for this was in a third party lib as well. |
I guess this will happen more often as more users try to use preact. There's a workaround. I can defer the call to onChildrenIsMounted using setTimeout(..., 0). It's a hack, but not super-terrible I guess... So far I only had 3 issues porting my App to preact (#551, this one here, and another one I'll be investigating later). Thanks for sharing this lib! |
Nice! I was going to suggest the setTimeout hack, but I didn't know if it would fix your usecase. Glad there's at least a workaround for now. We'll get #551 solved, that was just an oversight on my part. |
I think this is one of the big reasons I'm unable to switch a big application over to preact is the fact that I've been building a little react component over here: https://github.com/hanford/react-drag-drawer and lines like this break. In react, Any workarounds or ideas? |
I would definitely avoid invoking DOM side effects from within render like this - instead, use componentDidMount() {
if (this.state.open) {
this.attachListeners();
}
}
componentDidUpdate(prevProps, prevState) {
if (this.state.open && !prevState.open) {
this.attachListeners();
}
} Ideally, you want |
Great advice! Thanks again @developit |
Hi, I think I have similar issue but I didn't fully understand the conversation above. I have this test code: class Test extends Component {
setRef = (el) => {
console.log("Setting ref to " + el);
//if (!el) return;
this.el = el;
}
showA = () => {
this.setState({show: true});
}
closeA = () => {
this.setState({show: false});
}
showEl = () => {
console.log(this.el);
}
updateEl = () => {
this.el.textContent = new Date().getTime().toString();
}
renderA() {
const { show } = this.state;
if (!show) return null;
return (
<div>
A <button onClick={this.closeA}>Close A</button>
</div>
);
}
render() {
return (
<div>
{this.renderA()}
<div ref={this.setRef} />
<button onClick={this.showA}>Show A</button>
<button onClick={this.showEl}>Show el</button>
<button onClick={this.updateEl}>Update el</button>
</div>
);
}
} Reproducing steps: click
So after closing A element my ref callback is being called twice and second time with null, so I can't use DOM element anymore. If I uncomment the Could you please explain this behavior? What is best way to store correct reference to DOM element in Preact? |
@Kagami you're missing a // render after clicking "Show A":
<div>
<div>A <button onClick={this.closeA}>Close A</button></div> <!-- from renderA -->
<div ref={this.setRef} />
<button onClick={this.showA}>Show A</button>
<button onClick={this.showEl}>Show el</button>
<button onClick={this.updateEl}>Update el</button>
</div>
// render after clicking "Close A":
<div>
<div ref={this.setRef} />
<button onClick={this.showA}>Show A</button>
<button onClick={this.showEl}>Show el</button>
<button onClick={this.updateEl}>Update el</button>
</div> Since there's nothing to indicate which div is which, it's removing the second one instead of the first. Making Cheers! |
I managed to get react-toolbox to work with Preact (which was mostly a matter of customizing the Webpack config). I've got one remaining problem that is due to some internal differences between React and Preact VDom libraries. From what I've gathered, components should avoid using node references is componentDidMount or componentDidUpdate without wrapping them in a `setImmediate` or `setTimeout(…, 0)` (see preactjs/preact#477 ). With the current logic of componentDidUpdate in TimePickerDialog.js, the component throws a `TypeError: Cannot read property 'handleCalculateShape' of undefined` and interaction is impossible: Fixing this in Preact seems like a difficult if not impossible task, however fixing it in the TimePicker component is only a matter of wrapping the call to `this.clockNode.handleCalculateShape`in an arrow function so that this.clockNode is not accessed immediately on component update. Let me know if the change is acceptable :-)
I managed to get react-toolbox to work with Preact (which was mostly a matter of customizing the Webpack config). I've got one remaining problem that is due to some internal differences between React and Preact VDom libraries. From what I've gathered, components should avoid using node references is componentDidMount or componentDidUpdate without wrapping them in a `setImmediate` or `setTimeout(…, 0)` (see preactjs/preact#477 ). With the current logic of componentDidUpdate in TimePickerDialog.js, the component throws a `TypeError: Cannot read property 'handleCalculateShape' of undefined` and interaction is impossible: Fixing this in Preact seems like a difficult if not impossible task, however fixing it in the TimePicker component is only a matter of wrapping the call to `this.clockNode.handleCalculateShape`in an arrow function so that this.clockNode is not accessed immediately on component update. Let me know if the change is acceptable :-)
We completely rewrote the |
import React from 'react';
import Menu from '../icons/Menu.png';
export default class IconButton extends React.Component{
constructor(){
super();
this.icon = null;
this.login = null;
this.handleMouseOver = this.handleMouseOver.bind(this);
}
handleMouseOver(toggle){
console.log("handleMouseOver Called");
let background = "#034d79";
let foreground = "white";
if(toggle === true){
background = "white";
foreground = "#034d79";
}
console.log("icon: "+this.icon);
if(this.icon != null){
this.icon.setAttribute("background-color", background);
this.icon.setAttribute("color", foreground);
}
if(this.login != null){
this.login.setAttribute("background-color", background);
this.login.setAttribute("color", foreground);
}
}
render(){
return (
<div align="right" className="div-inline">
<img src={Menu} alt="Icon" name="icon" align="center" ref={node => {this.icon = node;}} onMouseEnter={this.handleMouseOver(true)} onMouseLeave={this.handleMouseOver(false)} className="login-icon"></img>
<button name="login" align="right" ref={node => {this.login = node;}} onMouseEnter={this.handleMouseOver(true)} onMouseLeave={this.handleMouseOver(false)} className="login-button">Login</button>
</div>
);
}
} The ref holder variable shows null! ? Could someone help me understand why this null error is? Can I set the attributes like this? |
@Kamarudeen-k you are calling your mouse handler functions in render() when you should only be passing references to them. This has nothing to do with refs, it's a mistake in your JavaScript code. - <foo onClick={this.foo(true)}>
+ <foo onClick={this.foo.bind(this,true)}> Here is the corrected code: return (
<div align="right" className="div-inline">
<img src={Menu} alt="Icon" name="icon" align="center" ref={node => {this.icon = node;}} onMouseEnter={this.handleMouseOver(true)} onMouseLeave={this.handleMouseOver(false)} className="login-icon"></img>
<button name="login" align="right" ref={node => {this.login = node;}} onMouseEnter={this.handleMouseOver.bind(this,true)} onMouseLeave={this.handleMouseOver.bind(this,false)} className="login-button">Login</button>
</div>
); |
Thank you for your quick response. yes, It works with its reference |
Not sure if this is supposed to work this way or not, but it seemed odd to have the ref only work sometimes.
I've got a div with a ref callback, and using that ref I wanted to get it to initialize swipejs which requires a width from the ref's children. However, I found that the ref is sometimes not yet part of the DOM. So when it tries to access the width on the page within the ref callback sometimes the width is zero, and sometimes it is my expected width of ~1000px. I dug in, and saw the width being 0,
document.querySelectorAll('.swipe')
being[]
, andref.parentNode
beingnull
during the times it wasn't working. Other times those all returned expected results of the width, the ref, and the parent.Didn't have time to dig in too deep, but I did find a workaround by just grabbing the ref in the callback, and then using that ref in the componentDidMount lifecycle hook.
Here's my code with the working workaround
The text was updated successfully, but these errors were encountered: