Forwarding ref returns "uninitialized" element

[xehpuk]

Describe the bug

I have this component:

function Div(props) {
    return (
        <div {...props}/>
    )
}

And I use it like this:

<Div id="hello" ref={el => console.log('id', el.id)}>Div</Div>

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-edibnb?file=src%2FApp.jsx

Steps to Reproduce the Bug or Issue

Use the Div component and look at the console output. It shows:

id <empty string>

Expected behavior

I expected to have access to the element's attributes, e.g. id. Instead, I seem to get default attributes ("" instead of "hello").

Screenshots or Videos

No response

Platform

• OS: Windows
• Browser: Firefox
• Version: 120.0.1

Additional context

Workaround: Wrap the ref inside onMount().

[lxsmnsyc]

issue seems to come from here: https://github.com/ryansolid/dom-expressions/blob/0aeca28611b5e9d85b135efbfc39892672efac95/packages/dom-expressions/src/client.js#L178-L179

Not sure if swapping them would fix the issue

[xehpuk]

@lxsmnsyc If I understand correctly, the code you referenced only affects {...props}. But this issue also occurs for "normal prop propagation":

function Div(props) {
    return (
        <div id={props.id} ref={props.ref}>{props.children}</div>
    )
}

[takeramagan]

@xehpuk @lxsmnsyc I don't know why, it seems that the ref to your custom Div component is called asynchronously.
But the ref to div is called synchronously
if you change the ref function to:

function ref(el) {
    setTimeout(()=>{
      console.log('id', el.id);
    })
}

Then it works.

[lxsmnsyc]

@takeramagan ref functions are assigned synchronously yes. Usually we'd recommend doing this inside an effect.

[ryansolid]

Yeah this was a conscious decision to allow synchronous passthrough. This was debated a lot pre-1.0 release and stayed here. But I'm more than happy to talk about this again for 2.0.

[LexSwed]

This was debated a lot pre-1.0 release

Is this debate saved somewhere, to see what was the reasoning for the future debate?

[ryansolid]

Like most things it was in the discord. Let me see if I can find it. I am definitely regretting losing that in there. Looking it appears that I was incorrect. The discussion before 1.0 was whether to null them onCleanup. This question came up post 1.0 in legacy help channel (pre-threads) and is pretty painful to read through (and is missing some stuff in my suggestion). I ended up summarizing my thoughts in a private channel though:

— 08/13/2021 2:17 PM
⁠The problem is that people who use the callback form(or directives) may not receive fully realized elements
It's a tricky tradeoff.. since if you wish to pass refs to children you expect them to be there first.. so changing that could cause issues.. similarly things like select expect the children to be there before setting the value..
So it's a mess and I need to decide on the correct order and stick to it. If it is wrong I want to fix it ASAP because it is subtly breaking or changes our communication. Does anyone have thoughts about this?

I mean depending on the order for refs could make it that you always need to put them in a Signal if you want to pass them out of local scope. I think the order is:

// Today:
1. Static
2. Refs
3. Children
4. Dynamic

// Or:
1. Static
2. Children
3. Dynamic
4. Refs

I think after seeing select children before dynamic makes sense.. I just can't figure out if we should fully guarentee fully realized refs or keep the sort of natural flow order we have now. It would mean child refs would be called before parent refs in the latter flow unless we try to schedule them which has more overhead. The more I think about some order is going to be unexpected for someone. It's possible it's fine how it is and we own it. And say use onMount if you want everything resolved.

[geohuz]

I recently encoutered similar problem in a async loaded component, in this case the ref is always undefined even put into onMount. I don't know the best practise of accessing the ref here. Currently I created signals and passing function prop from parent to the child and calling the callback function inside the child component to trigger state change in the parent component.
I've tested that in this case the setTimeout trick works