-
Notifications
You must be signed in to change notification settings - Fork 311
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
[JSX] Hack to allow plugins to specify a property is to be spread rather than constructed as a key,value pair #4038
base: main
Are you sure you want to change the base?
Conversation
This should be fine. Maybe using |
|
@alfonsogarciacaro This PR is ready with tests; I will be able to produce tests for #4037 after this is merged as the tests will not work with #4037 (so I think) because there will not be an open/closed tag with #4037 |
@shayanhabibi Thank you for opening a PR. One flow I see with the current proposition is that only 1 argument can use this system as at a time. What I mean by that is what if a Using an attribute to decorate the argument would allow such scenario, I am just not sure if the information is available in the AST at this point. [<JSX.Component>]
let CounterJSXMagic(
[<JSX.SpreadProperty>] foo: obj,
[<JSX.BooleanProperty>] required : bool,
[<JSX.BooleanProperty("my-property-name")>] bar : bool
) =
JSX.html $"<div></div>" |
As it currently stands, this is to enable plugins (specifically Oxpecker.Solid) to provide this functionality via some other API. There is no easier way to produce these type of attributes within the current JSX generation/render phases otherwise without some deeper overhaul afaict. UsageIn the example of Oxpecker.Solid, specific method usage is transformed into property expressions with the property key set to [<SolidComponent>]
let Button ( so : SomeObject ) ( so2 : SomeObject ) =
button(class'="someclass").spread(so).spread(so2) { "I'm the button text" } <button class="someclass" {...so} {...so2}>I'm the button text</button> TestI stropped the magic properties in the test just to check that providing the magic key produces the desired outcome rather than that being the desired API usage for the functionality :P |
Hum ok I see. In theory, if we were to use an code from your branch on Oxpecker | "spread", { Args = [ _; identExpr ] } -> ("__SPREAD_PROPERTY__", identExpr) :: restResults My issue, is that in general when we start accepting a "hack" solution then often it become a stable supported in the future because people start to use it. IHMO, if we can make an implementation which don't rely on a hack it is for the best. Because, here we kind of cheat the F# type system because we are after it and Fable is kind enough to not do any check on the properties uniqueness :) I didn't load the projects in an IDE so I don't know if have the information required to use attributes and how much work it would be to do that instead. |
I'm not at all against that ideology. To allow this same functionality to be used in the std Fable JSX seems doable. Spread Prop[<JSX.Component>]
let Button (prop : obj) ([<ParamList>] someObj : obj) = // .... I imagine the Bool Prop[<JSX.Component>]
let Button (prop: obj) (required: JSX.PropBool) (someOtherBoolProp: JSX.PropBool) = //... I Imagine a type alias to a boolean provided within Fable could be used to trigger a branch in the printer/renderer specific for boolean properties. The name of the identifier can then be directly translated to the attribute key. Edit: In terms of being able to use it with the Oxpecker.Solid DSL/Plugin, I'm sure I'll figure something out :P. |
Bool propType aliases disappear in Fable AST, but if this general JSX behavior, we can just skip printing the value if it's a Spread PropFor the call site, we can indeed access the argument attribute and use for example Fable/src/Fable.Transforms/Fable2Babel.fs Lines 2064 to 2066 in 5c95930
My main concern is that, unless this is a feature only for declaring bindings to native JS components, resolving this in the declaration site requires, unless I'm mistaken, to flatten the spread props with the other props. This is trivial for records but other types maybe trickier. |
This would work for HTML5, where the absence of a attribute means its 'bool' value is Essentially, the absence of an attribute in JSX does not equate to What this means, is that the absence of the attribute does not necessarily mean the same thing as providing the attribute a value of false. However, the presence of the attribute will equate to the attribute having a value of true. To match this behavior, a This might not be something actually worth addressing, or enabling.
This is a functionality which hasn't been needed before, and I don't think is something worth addressing within Fable either, besides providing authors the ability to enter unsafe territory on their own terms and stipulations. It is something which I want, to be able to directly translate and port JSX to F# DSLs. It is for declaring bindings to native JS components, but also to manipulate properties that are passed down component hierarchies when utilising headless libraries. Boolean attributes don't function differently if used with boolean values in JSX (unless the UI library authors are sadistic. Material UI uses |
I'm a bit stuck at this point, I'm passing the attributes down the transformation tree, but I'm not sure how to approach it from here. Is there some way to tag or pass an attribute into the Expression type? Edit: I see this breaks things in another call tree so this isn't an approach that would work. I keep coming back to hijacking the Printer attribute name :') |
Bool propAh, ok! Actually I just realized we are already skipping the prop if the value is null or undefined, so maybe we print the prop name only when the value is a Spread propThere's no standard way to pass info to the Babel printer, but we could add something to the JsxElement in the Babel AST. Another "hack" I was thinking of besides the magic words is to allow |
This additional, non-obstructive branch statement in the JSX adds functionality that is particularly needed in my case to spread objects in JSX.
A plugin can provide an invalid attribute name "SPREAD_PROPERTY" in a property list to identify to the printer to skip the key and print the value enclosed in an object spread
{...value}
.Just spit balling on how to implement it, but I would really like to push for something along these lines to work in JSX compilation.