React19 and key not found warning

[Freymaurer]

Using Feliz with React 19 return a lot of missing key warnings. I investigated this issue and showcase the problem, as well as some solutions

The Issue

So i took quite some time to get to the bottom of this issue and the culprit is Interop.reactApi.Children.toArray using the react Children api.

The Children.toArray function calls the following, which showcases why we now get these warnings. It still recreates the react element with automatic keys .0, .1..., but we get the warning.

`toArray` subfunction

if (__DEV__) {
// If `child` was an element without a `key`, we need to validate if
// it should have had a `key`, before assigning one to `mappedChild`.
// $FlowFixMe[incompatible-type] Flow incorrectly thinks React.Portal doesn't have a key
if (
  nameSoFar !== '' &&
  child != null &&
  isValidElement(child) &&
  child.key == null
) {
  // We check truthiness of `child._store.validated` instead of being
  // inequal to `1` to provide a bit of backward compatibility for any
  // libraries (like `fbt`) which may be hacking this property.
  if (child._store && !child._store.validated) {
    // Mark this child as having failed validation, but let the actual
    // renderer print the warning later.
    newChild._store.validated = 2;
  }
}

Solution 1: Spreading children

Looking at the documentation for React.createElement, we can find the following parts:

createElement(type, props, ...children)

optional ...children: Zero or more child nodes. They can be any React nodes, including React elements, strings, numbers, portals, empty nodes (null, undefined, true, and false), and arrays of React nodes.

You should only pass children as multiple arguments to createElement if they are all statically known, like createElement('h1', {}, child1, child2, child3). If your children are dynamic, pass the entire array as the third argument: createElement('ul', {}, listItems). This ensures that React will warn you about missing keys for any dynamic lists. For static lists this is not necessary because they never reorder

Currently Feliz handles children by passing them always as props. Before React 19 this was sufficient using the Children.toArray method. We can adjust the createElement function in Feliz to spread children as third optional parameter:

[<Erase>]
type React =

  [<ImportMemberAttribute("react")>]
  static member inline createElement (
      _type: string, 
      props: obj, 
      [<ParamArray>] children: ResizeArray<ReactChild>) : ReactElement = jsNative

By doing this we tell react that everything is handled statically and no missing-key-warning gets emitted.

The functions reactElementWithChild, reactElementWithChildren, createElement can be rewritten accordingly:

`reactElementWithChild`, `reactElementWithChildren`, `createElement`

module ReactHelper =
  let inline reactElementWithChild (name: string) (child: 'a) =
    React.createElement(
      name, 
      createObj [ ],
      ResizeArray([!!child])
    )

  let inline reactElementWithChildren (name: string) (children: #seq<ReactElement>) =
      React.createElement(
        name, 
        createObj [],
        !!children
      )
  let createElement name (properties: IReactProperty list) : ReactElement =
    match properties |> List.partition (fun (key, _) -> key <> "children") with
    | props, ["children", children] ->
      React.createElement(
        name, 
        createObj props,
        !!children
      )
    | props, _ -> 
      React.createElement(
        name, 
        createObj props,
        ResizeArray()
      )

...and prop.children..:

`prop.children`

[<Erase>]
type prop =

    /// Children of this React element.
  static member inline children (value: ReactElement) : IReactProperty = 
    "children", value
  /// Children of this React element.
  static member inline children (elems: ReactElement seq) : IReactProperty = 
    "children", elems

Advantages

• No legacy Children api
• No element rebuilding
• no fable/feliz magic creating keys where no keys are written by the developer (i actually started with fable react and when i wrote my first js/ts react i was confused why i should write keys for all lists of elements)

Disadvantages

• No key warnings for dynamic lists

We cannot know if the ReactElement list was created only with static elements or with dynamic elements inside

Example

type Test =

  [<NamedParamsAttribute>]
  static member CreateReact(?init: int) : ReactElement =
    let init = defaultArg init 0
    let (count, setCount) = React.useState init
    let increment = fun () -> setCount (count + 1)
    let decrement = fun () -> setCount (count - 1)
    Html.div [ 
      Html.div [ // static
        prop.text count
        prop.key 1
      ]
      Html.button [ prop.text "Increment"; prop.onClick increment ] // static
      Html.button [ prop.text "Decrement"; prop.onClick decrement ] // static
      for i in 0 .. count do // dynamic
        Html.div i
    ]

Solution 2: Custom .toArray function

The react Children.toArray function can be broken down to:

1. Take existing element (exposes props and keys as members!)
2. Create new element using these with an adjusted key

We can do this ourselves!

Below you can find the code for types and cloning ReactElement with a new key:

Details

[<ImportMember("react")>]
type [<AllowNullLiteral>] ReactElement =
    abstract member key: string with get
    abstract member ``type``: string with get
    abstract member props: obj with get
    abstract member ref: obj with get
    abstract member _owner: obj with get
    abstract member _store: obj with get

module ReactHelper =

  open Fable.Core
  open Fable.Core.JsInterop

  let spreadPropsWith(currentProps:obj, newProps: obj) =
    emitJsStatement (currentProps, newProps) """
      return { ...$0, ...$1 };
    """
    
  let cloneAndReplaceKey (element: ReactElement, newKey: string) =
    let props = element.props
    if isNullOrUndefined element.key then
      let newProps = spreadPropsWith(props, createObj [|"key", newKey|])
      React.createElement(element.``type``, newProps, ResizeArray())
    else
      element


  let autoCreateIndices (children: #seq<ReactElement>) =
    [|
        for i in 0 .. (Seq.length children) - 1 do
          cloneAndReplaceKey(Seq.item i children,sprintf "auto%i" i)
    |]

autoCreateIndices (children: #seq<ReactElement>) : ReactElement [] can now be used in prop.children and reactElementWith Children to automatically index all ReactElements.

Advantages

• Recreates logic used in React 18, again all elements without a key get a key assigned.

Disadvantages

• Pseudo clones the react elements (which is - as far as i understand Children.toArray - also currently done)
• Fable/feliz magic adding keys the developer did not add themselves

Solution 3: prop.nested for static children

This idea was proposed by @Canna71 here

In addition to prop.children, add prop.nested.

Nested will be used as spread argument for React.createElement (will be static), children will be given as props.

Advantages

• User can decide what to use

Disadvantages

• Adding a feliz specific property
• Unable to define static and dynamic children in a mixed order. As you could have no option to mix them.

Say you want to do:

Html.div "Hello World" //static
for i in 0 .. 10 do
  Html.div i //dynamic
Html.div "ByeBye!" //static

This would not be possible.

Solution 4: Computational Expression

Using a CE would allow us to directly feed different ReactNode types into our function.

From the official docs we can see that React nodes can be a lot of things

optional ...children: Zero or more child nodes. They can be any React nodes, including React elements, strings, numbers, portals, empty nodes (null, undefined, true, and false), and arrays of React nodes

We can represent this using f# computational expressions with different _.Yield members:

CE builder

type ReactBuilder(name: string) =
    member inline _.Yield(text: string): BuilderFun -> unit =
      fun (builder: BuilderFun) -> 
        builder !!text

    member inline _.Yield(children: ReactElement list): BuilderFun -> unit =
      fun (builder: BuilderFun) -> 
        builder (U2.Case1 !!children)

    member inline _.Yield(child: ReactElement): BuilderFun -> unit =
      fun (builder: BuilderFun) -> 
        builder (U2.Case1 !!child)

    member inline _.Combine([<InlineIfLambdaAttribute>] first: BuilderFun -> unit, [<InlineIfLambdaAttribute>] second: BuilderFun -> unit): BuilderFun -> unit =
      fun (builder: BuilderFun) ->
        first builder
        second builder

    [<Erase>]
    member _.Run(a: BuilderFun -> unit) : ReactElement =
      let children = ResizeArray()
      let props = ResizeArray()
      let builderFun (x: U2<ReactChild, IReactProperty>) : unit = 
          match x with
          | U2.Case1 child -> children.Add child
          | U2.Case2 prop -> props.Add prop
      a builderFun
      React.createElement(name, props, children)

    member inline x.Delay([<InlineIfLambda>] f: unit -> (BuilderFun -> unit)) = f()

As computational expressions are only a (from my understanding) sequence of functions i decided to return a function from all Yields. This function takes whatever the input of the yield was and applies a to unit function on it. Then in _.Run this function is called and the different Yield inputs are sorted into two ResizeArrays.

let main () =
    Html.div {
        "Hello World"
        Html.div { "Nested Div" }

        [
            for i in 0..10 do
                Html.div { "Item " + i.ToString() }
        ]
    }

export function main() {
    let second_1, first, child, second, children;
    return ReactBuilder__Run_Z6C13D325(Html_div, (second_1 = ((first = ((child = ReactBuilder__Run_Z6C13D325(Html_div, (builder_1) => {
        builder_1("Nested Div");
    }), (builder_2) => {
        builder_2(child);
    })), (second = ((children = toList(delay(() => map((i) => {
        let text_2;
        return ReactBuilder__Run_Z6C13D325(Html_div, (text_2 = ("Item " + int32ToString(i)), (builder_3) => {
            builder_3(text_2);
        }));
    }, rangeDouble(0, 1, 10)))), (builder_4) => {
        builder_4(children);
    })), (builder_5) => {
        first(builder_5);
        second(builder_5);
    }))), (builder_6) => {
        builder_6("Hello World");
        second_1(builder_6);
    }));
}

There are two flavors to this. One is doing a Oxpecker approach in doing div( ..props ) { ..children }, the other is the onve shown above.

As the js code is not perfect, this solution could be improved by writing a CompilerPlugin similiar to Oxpecker (with createElement or JSX return value.

Solution 5: JSX?

This is a big questionmark, as it is propably more breaking than the other solutions (requiring fable to transpile to a different file extension).

Using the code written by Alfonso in Feliz.JSX, Feliz could move to a full JSX syntax. There is still some issues on fable compiler side.

End

Thats it, would love to help out with this, as i would like to move my app to react 19.

Thanks for @MangelMaxime for providing feedback during the investigation step!

[MangelMaxime]

This issue is related / duplicate of #21

But having a clean history for the discussion is perhaps for the best as in #21 it got side tracked, and not much information from it.

Actually, it is perhaps not a duplicate or if it is it's probably more one from #607

---

IHMO, if we can have a quick fix by mimicking the old behavior this is perhaps for the best short term. Which seems to be solution 2 because disabling totally the warning from React (solution 1) doesn't seems like a good idea.

Long term, I would definitely want Feliz to emit JSX instead of the compiled result of JSX. Reason for that is to avoid these kind of issues and also to allow using the new JSX runtime API. Babel REPL

This is needed to get the latest optimization from React ecosystem and new stuff from react-native.

I am not sure yet if the move to JSX should happen in Feliz itself or an external library which mimic Feliz API but would also support others libraries. JSX by itself is not specific to a library, and so if we can make it plug-gable we could share the common HTML API between frameworks which could be nice to have.

[Canna71]

@Freymaurer Thank you for listing my proposal.
I would prefer to not generate keys automatically in general, because this would prevent the warning when it is actually useful (for example in sortable lists where the key is an actual identifier, but we forgot to map it to the React Element).
So JSX would be a solution in that it is explicit when the children are actually 'static' and so a autogenerated key is of course OK.
The only cons is that it needs JSX and it is - in some way - not a "Feliz" solution but a Fable one.
My proposal (which is working) aims at explicitely state to Feilz that what we are creating is a nested structure, and not a list of elements. For other lists, I would prefer to keep receiving the warning if I forget to set the keys myself.
Mixing static and non static content should be possible (like in standard React) using fragments, that is, inserting the dynamic list inside a fragment. (Still haven't tried though).

[Freymaurer]

I agree with your core idea. There should be minimal magic involved and the code should behave as it does in react. Surpressing key warning as the Children.toArray function did in React 18, might create ambiguity, especially for newer developers.

In the question of feliz showing the warning even for static list elements, the proposed solutions can be broken down into:

• do not show warnings
• change api
• Use JSX

[Canna71]

@Freymaurer I can confirm that using prop.nested solution allows for mixed approach:

  Html.div [
    prop.id "test"
    prop.name "test"
    prop.nested [
      Html.p "First child"
      React.fragment (["1";"2";"3"] |> List.map Html.p)
      Html.p "Last child"
   ]
  ]

will render as:

<div id="test" name="test">
  <p>First child</p>
  <p>1</p>
  <p>2</p>
  <p>3</p>
  <p>Last child</p>
</div>

With no warning, of course.
When using prop.children or inserting elements as direct "arguments" of the parent, will still generate warnings if no key is provided.

The resulting code is still quite compact:

export function test(state, dispatch) {
    let elems, xs;
    const props_3 = createObj(ofArray([["id", "test"], ["name", "test"], (elems = [createElement("p", {
        children: ["First child"],
    }), (xs = map( (children) => createElement("p", {
        children: [children],
    }), ofArray(["1", "2", "3"])),
    react.createElement(react.Fragment, {}, ...xs)), createElement("p", {
        children: ["Last child"],
    })],
    ["nested", reactApi.Children.toArray(Array.from(elems))])]));
    const nested = props_3.nested || [];
    delete props_3.nested;
    return createElement.apply(null, ["div", props_3, ...nested]);
}

[Freymaurer]

@Zaid-Ajaj @MangelMaxime any direction for this issue? Would really love to resolve this

[Canna71]

I ended up playing with my own version, have a look if you'd like, guys:

https://github.com/Canna71/FreeAct

[MangelMaxime]

@Zaid-Ajaj @MangelMaxime any direction for this issue? Would really love to resolve this

For everyone, here is what is happening being the scene at the moment:

1. I am trying to reach out to Zaid to see if we can help him maintain Feliz project by adding more maintainers. I have the idea of creating an equivalent to fsprojects for Fable. Because, I keep taking under my name core projects from the community but I will need help with maintaining them, plus it will allows us to try build an ecosystem where we can rotate maintainers more easily if needed.

2. In Fable 5, we are working on optimising the JSX output to be able to re-use Feliz API, but by emitting JSX directly which will resolve this issue in the best way possible (IHMO).

   Emitting JSX directly, means that we will delegate all the JSX transpilation to native JavaScript tools and just not care about how an list should not be written.

[Zaid-Ajaj]

I am trying to reach out to Zaid

@MangelMaxime I would be happy if more people helped me with the maintenance ❤ Especially these days, having almost no time for any OSS work but trying to review, merge and publish packages when I can. Any ideas on how we could make this happen? Do I share package ownership or share nuget API keys for publishing?

[MangelMaxime]

@Zaid-Ajaj

It depends on what you are okay with.

What I would like to propose is:

1. I create a fable-community organisation
2. We move Feliz to that organisation
3. I/We try to find people from the community interested in contributing I know @Freymaurer is

The reason for moving it under an organisation is because when we have a repo under a personal name, we can't give full control over the repo (for example only you can manage secrets and env variables). Under an organisation, I can create groups with different permissions, allows several people to manage secrets, etc.

Do I share package ownership or share nuget API keys for publishing?

I think this is better to share the ownership, as private keys can expire but ownership is for ever (unless you decide to revoke it).

For such things, my vision is to create a Fable Community organisation, where only a select few are invited in it. So if anything happens, we can have several people having access to the projects (this is what we did for Fable packages and it allowed me and others to keep contributing even when Alfonso stepped down).

If someone want to maintain a specific packages, we can also invite him has an owner under its own name to not give full control of all the packages.

This structure is similar to how fsprojects works for "standard" F# projects.

All this is a suggestions, and I don't want to impose/force anything on you.

[Zaid-Ajaj]

The reason for moving it under an organisation is because when we have a repo under a personal name, we can't give full control over the repo (for example only you can manage secrets and env variables). Under an organisation, I can create groups with different permissions, allows several people to manage secrets, etc.

@MangelMaxime

Although I would love to keep the repo here, I think this is the right direction for the project and its continuity

Alright! let's go ahead with the new org 🙏

I think this is better to share the ownership, as private keys can expire but ownership is for ever (unless you decide to revoke it).

That sounds good too! I need to figure out how to do that 😄

[MangelMaxime]

@Zaid-Ajaj Can you please make a Transfer request to move this repo to fable-hub? (Fable community was already taken..)

Although I would love to keep the repo here, I think this is the right direction for the project and its continuity

I would have liked to do that, but unfortunately we can't give admin/owner equivalent right to someone else on a personal Repo.

The reason for moving it under an organisation is because when we have a repo under a personal name, we can't give full control over the repo (for example only you can manage secrets and env variables). Under an organisation, I can create groups with different permissions, allows several people to manage secrets, etc.

@MangelMaxime

Although I would love to keep the repo here, I think this is the right direction for the project and its continuity

Alright! let's go ahead with the new org 🙏

I think this is better to share the ownership, as private keys can expire but ownership is for ever (unless you decide to revoke it).

That sounds good too! I need to figure out how to do that 😄

In order, to add another Owner, you need to:

1. Connect to Nuget
2. Go to your package page
3. Click on Manage package on the menu in the right
4. Click on Owners
5. Add me MangelMaxime

[Zaid-Ajaj]

Hi there @MangelMaxime, can you add me to the fable-hub org? I tried transferring the repo to it but it errored out with "cannot create public repository on the org".

Will add you as an owner on nuget as well

[MangelMaxime]

Hi there @MangelMaxime, can you add me to the fable-hub org? I tried transferring the repo to it but it errored out with "cannot create public repository on the org".

Sorry about that, I sent you an invitation

[Zaid-Ajaj]

@MangelMaxime I've moved the repo and added you as a nuget package owner. Thanks a lot for doing this and sorry for the delay 🙏 ❤