ryansolid
only assign ref to a variable
This fixes a bug where we generate ref assignment props.ref = or anything that is assignable (left value) which was also wrongly followed it in dom-expressions client.js (by me).
After fixing the workflow, I see there are some usages that will be broken by this PR.
https://github.com/ryansolid/dom-expressions/blob/45d6ad28f919160d6a16b2924385745718aa0c7c/packages/dom-expressions/test/dom/events.spec.jsx#L8-L12
There's also a question for class components this.ref although it doesn't work well
let a = new A()
<a.render />
createComponent(a.render, {}) // `this` won't be passed so `this.ref` doesn't make sense
Anything related to this @trusktr?
Well original thinking was that props.ref if provided would always get transformed into a function. So then we could always do a function check to make the decision. Which suggests that the fix for #316 may have not been correct. Which is why I see #333 reverting it..
However, I'm gathering that doesn' fix the original issue https://github.com/solidjs/solid/issues/2136. So you are attempting to do this by having the compiler be smarter about when it tries to assign stuff vs making it a function?
I guess the problem with the original code approach is it basically assumed there were no harm in assigning the ref element to props that was never getting it passed in anyway. Since there was no consumer on the otherside who cares. And our heuristics are insufficient to determine all the other cases.
My gut is we just merge #333 to revert the change since it appears the original fix broke stuff for the reporter anyway. And then we try to find better heuristics.
As far as this stuff, if createComponent(a.render, {}) is the generated code, then yeah I think it would be convenient for it to be called with this as the instance, f.e. createComponent(a.render.bind(a), {}), which I think is reasonable because its not possible to write <a.render() /> syntactically. A workaround is render = () => {} in the class.
