Skip to content
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

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

shayanhabibi
Copy link
Contributor

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.

@alfonsogarciacaro
Copy link
Member

This should be fine. Maybe using __SPREAD_PROPERTY__ instead? As we often surround magic words with two underscores. But it's not a big deal so if the plugin is expecting _SPREAD_PROPERTY_ it can go as is.

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 9, 2025

__SPREAD_PROPERTY__

Permits a value to be wrapped in obj spread syntax within JSX elements within the already defined property key,value schema.

ie, a property key,value pair of "__SPREAD_PROPERTY__", myProperties for some tag Tag will transpile to <Tag {...myProperties} />

This can cause undefined behaviour, exceptions, or errors if used on invalid objects within the JSX spec, so care must be taken by the user.

__BOOL_PROPERTY__

Based on mixed outcomes from different environments between some-attribute=true some-attribute="true" some-attribute={true} and some-attribute, there appear to be cases where one may cause issue over another.

Web components in particular, can use attributes as boolean switches, whereby the existence of the attribute key is truthy. This means some-attribute='', some-attribute="true", some-attribute="false" and some-attribute all result in a truthy outcome.

For completeness, I believe also adding a magic attribute key to output a property without any associated value is also worthy to consider in this PR.

ie, a property key,value pair of "__BOOL_PROPERTY__", "some-value" for some tag Tag will transpile to <Tag some-value />

MAGIC PROP KEYS

In general I imagine these will not be consumed directly by users of fable, but be made available as features through libraries and other DSLs.

Help wanted

Honestly, I'm not sure how to approach making tests in this environment for these kind of cases within the fable schema. I've scrolled through the React test framework, but I can't put together how the tests would function vis-a-vis compiling and comparing compiled output to provide flexibility in edge cases for plugins/users without polluting the test space with more libraries etc.

Would anyone be able to provide a sample that I can expand upon?

Documentation

I would like to provide documentation regarding these magics. Is there any central approach for documenting these for users and plugin writers?

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 9, 2025

@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

@MangelMaxime
Copy link
Member

MangelMaxime commented Feb 9, 2025

@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 JSX.Component wants to use 2 __BOOL_PROPERTY__ ? With the current approach I think this is not possible.

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>"

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 9, 2025

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.

Usage

In the example of Oxpecker.Solid, specific method usage is transformed into property expressions with the property key set to __SPREAD_PROP__. There is no limit to how many times this key can be provided by a plugin in a property key,value pair afaik.

[<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>

Test

I 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

@MangelMaxime
Copy link
Member

As it currently stands, this is to enable plugins (specifically Oxpecker.Solid) to provide this functionality via some other API.

I 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 Attribute it could perhaps be possible to change the identExpr instead of the property name in

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.

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 9, 2025

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 ParamListAttribute could be used to indicate to the printer to handle the property in some manner. I'm not sure how to implement this effectively, but I could do some fiddling.

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:
Ah I see that outside a plugin environment, I can do attributes for the properties (as you suggested before)
I'll go about this tomorrow/day after. Thank you!

In terms of being able to use it with the Oxpecker.Solid DSL/Plugin, I'm sure I'll figure something out :P.

@shayanhabibi shayanhabibi marked this pull request as draft February 9, 2025 17:21
@alfonsogarciacaro
Copy link
Member

Bool prop

Type aliases disappear in Fable AST, but if this general JSX behavior, we can just skip printing the value if it's a true literal (and skip the prop entirely if it's false).

Spread Prop

For the call site, we can indeed access the argument attribute and use for example ParamObject to define spread properties (we still need to pass this information to the printer somehow but changing the Babel AST is not a big problem because it's not exposed to plugins). Here:

let transformJsxCall (com: IBabelCompiler) ctx callee (args: Fable.Expr list) (info: Fable.MemberFunctionOrValue) =
let names =
info.CurriedParameterGroups |> List.concat |> List.choose (fun p -> p.Name)

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.

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 10, 2025

Bool prop

Type aliases disappear in Fable AST, but if this general JSX behavior, we can just skip printing the value if it's a true literal (and skip the prop entirely if it's false).

This would work for HTML5, where the absence of a attribute means its 'bool' value is false, while the presence of the attribute is true, but is actually different in JSX, see https://stackoverflow.com/questions/60816731/how-are-boolean-props-used-in-react

Essentially, the absence of an attribute in JSX does not equate to false, but actually equates to undefined, at which time a default value may be inserted.

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 true literal would result in skipping the printing of the value, but a false literal should result in a value of false. undefined would result in the property being skipped entirely.

This might not be something actually worth addressing, or enabling.

Spread Prop

For the call site, we can indeed access the argument attribute and use for example ParamObject to define spread properties (we still need to pass this information to the printer somehow but changing the Babel AST is not a big problem because it's not exposed to plugins). Here:

let transformJsxCall (com: IBabelCompiler) ctx callee (args: Fable.Expr list) (info: Fable.MemberFunctionOrValue) =
let names =
info.CurriedParameterGroups |> List.concat |> List.choose (fun p -> p.Name)

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 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 true|false|undefined to define the three states of the checkbox, where undefined is the third state), so i've never been left unable to achieve the same result in F# that is achieved in JSX. This is not true for spreading objects.

@shayanhabibi
Copy link
Contributor Author

shayanhabibi commented Feb 10, 2025

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 :')

@alfonsogarciacaro
Copy link
Member

Bool prop

Ah, 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 true literal.

Spread prop

There'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 Emit for the JsxProps. For example, when Fable2Babel finds the ParamObject attribute, it transforms the prop name to something like emit:{...$0} and BabelPrinter resolves it like other emit templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants