solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.17k stars 918 forks source link

Refs don't work when optional on a component #2137

Closed danieltroger closed 5 months ago

danieltroger commented 5 months ago

Describe the bug

I mentioned in https://github.com/solidjs/solid/issues/2136 I observed my refs not being called - turns out I was right. When forwarding a ref in a component, making that ref optional breaks refs in solid completely.

Your Example Website or App

https://playground.solidjs.com/anonymous/e3d19859-59ee-4811-93da-bbfd2bd74d1a

Steps to Reproduce the Bug or Issue

  1. Go to https://playground.solidjs.com/anonymous/e3d19859-59ee-4811-93da-bbfd2bd74d1a
  2. Check the console
  3. Nothing is logged - the ref is not being called
  4. Change props?.ref to props.ref
  5. Now something is logged

Expected behavior

props?.ref should also work in case one wants to "forward" a ref from an element created inside a component but doesn't always want to accept a ref

Screenshots or Videos

No response

Platform

SolidJS playground

Additional context

No response

ryansolid commented 5 months ago

To be fair the props object itself being optional probably not terribly common, but I could see some deeper object having this issue. But yeah.. the whole expression doesn't get processed it looks like. Compiler issue. Thanks.

danieltroger commented 5 months ago

Yeah I didn't realise that I probably don't need the optional chaining there? But some error message made me add it in my code (adding it made the error go away) and then refs stopped working which sent me on another debugging journey I would have preferred to not be on

SarguelUnda commented 5 months ago

I think I narrowed where the bug is but I don't know how to fix it:

https://github.com/ryansolid/dom-expressions/blob/ac40fb85d27d119ee940d3f555e668ebaeaea22f/packages/babel-plugin-jsx-dom-expressions/src/dom/element.js#L468

t.isLVal(value.expression) return true for props.ref but false for props?.ref

t.isOptionalMemberExpression return true for props?.ref but if I try to pass it in the if statement it bugs with

Property left of AssignmentExpression expected node to be of a type ["LVal"] but instead got "OptionalMemberExpression"

I don't know anything about babel so I'm not sure how to transform a OptionalMemberExpression in MemberExpression

mdynnl commented 5 months ago

pretty close.

props?.ref isn't a LHS value i.e. we can't do props?.ref = node unlike member expr props.ref. same for props.a?.b, hello(), cond ? c : d, or any expression that isn't literal or member can potentially return a function so the correct transform is

const $ref = props?.ref
typeof $ref === 'function' && use($ref, node)