Fable icon indicating copy to clipboard operation
Fable copied to clipboard

Inlining const React children

Open kerams opened this issue 2 years ago • 1 comments

Description

Consider a simple div with 2 elements

let f () =
    div [ Class "gg" ] [
        div [] []
        span [] []
    ]

which compiles to

export function f() {
    const children_4 = [react.createElement("div", {}), react.createElement("span", {})];
    return react.createElement("div", {
        className: "gg",
    }, ...children_4);
}

This is fine, but we're left with a few unnecessary bytes (maybe there are even performance ramifications due to spreading?) and Terser doesn't handle similar cases either (screenshot of pretty print)

image

Could Fable recognize these single use consts and emit

export function f() {
    return react.createElement("div", {
        className: "gg",
    }, react.createElement("div", {}), react.createElement("span", {}));
}

or

export function f() {
    return react.createElement("div", {
        className: "gg",
    }, [react.createElement("div", {}), react.createElement("span", {})]);
}

(Not sure if there's a difference between the two, in performance or otherwise)

Related information

  • Fable version: 4.5.0

kerams avatar Dec 18 '23 09:12 kerams

While looking at this issue, I learned for the first time about [<ParamList>] attribute.

According to the changelog and the Fable.Core doc comment, it seems to only exist for Fable.React...

Changelog

## 3.0.0-nagareyama-alpha-003 - 2020-09-14

* Fix: Add names with $reflection suffix to root scope
* Fix deriving from imported JS classes
* Keep support for ParamList attribute until removed from Fable.React

Fable.Core

/// Used to spread the last argument. Mainly intended for `React.createElement` binding, not for general use.
[<AttributeUsage(AttributeTargets.Parameter)>]
type ParamListAttribute() =
    inherit Attribute()

type ParamSeqAttribute = ParamListAttribute

Looking thought the code and after an empiric test, I am unsure what the effect of [<ParamList>] attributes is, looking through the code it seems to be of the same effect as when we use [<ParamArray>].

IHMO If that's confirmed it would be nice, to remove it in a future major version of Fable.

With that said, while testing what happen if we remove that attribute I discovered that the code should still be working without but it would not save the few extra bytes that you mentioned which is probably not what we want.

// With [<ParamList>]
export function f() {
    const children_4 = [react.createElement("div", {}), react.createElement("span", {})];
    return react.createElement("div", {}, ...children_4);
}

// Without [<ParamList>]
export function f() {
    const children_4 = [react.createElement("div", {}, []), react.createElement("span", {}, [])];
    return react.createElement("div", {}, children_4);
}

// With [<ParamArray>]
export function f() {
    const children_4 = [react.createElement("div", {}), react.createElement("span", {})];
    return react.createElement("div", {}, ...children_4);
}

Regarding the creation of the intermediate variable children_4, I am unsure how Fable decide to create it. What I mean by that is why Fable create a intermediate variable for children and not for the properties object {} in the exemple Without [<ParamList>].

MangelMaxime avatar Dec 18 '23 10:12 MangelMaxime