Zaid-Ajaj / Feliz

A fresh retake of the React API in Fable and a collection of high-quality components to build React applications in F#, optimized for happiness
https://zaid-ajaj.github.io/Feliz/
MIT License
534 stars 78 forks source link

ReactComponent attribute does not work without calling `ofFunction` where used #544

Open sonicbhoc opened 1 year ago

sonicbhoc commented 1 year ago

So I am having a strange issue with Feliz 1.68 and Fable 3.7.1.

I'm trying to author React components. When I just add the [<ReactComponent>] attribute to an F# function with a capital name and a list of properties (which is usually empty), I'm expecting it to generate a React Function Component. But the generated component's logic is essentially comprised of constructing arrays like what's in the F# code. Which is fine, except my React components don't work.

In every example I've seen, all I am supposed to need is the ReactComponent Attribute and supposedly I'd get a function component, and then I can just call that component like a function. But when I do that, I get this error:

Invalid hook call. Hooks can only be called inside of the body of a function component.

And the generated code looks like this:

export function Router() {
    let elements;
    const patternInput = Feliz_React__React_useState_Static_1505(RouterModule_urlSegments(window.location.hash, 1));
    const updateUrl = patternInput[1];
    const currentUrl = patternInput[0];
    return RouterModule_router(createObj(ofArray([["onUrlChanged", updateUrl], (elements = toList(delay(() => {
        const matchValue = Url_Parse_1334CEF1(currentUrl);
        switch (matchValue.tag) {
            case 1: {
                return singleton(createElement("h1", {
                    children: ["Logging In..."],
                }));
            }
            case 0: {
                return singleton(Index());
            }
            case 2: {
                return singleton(Estimator());
            }
            case 3: {
                return singleton(TimeTracker());
            }
            default: {
                return singleton(createElement("h1", {
                    children: ["Error 404: Not Found"],
                }));
            }
        }
    })), ["application", react.createElement(react.Fragment, {}, ...elements)])])));
}

export function PageContainer() {
    let elems_5, elms_1, elms, props_2, elems_2, elms_2, props_6, elems_3;
    const props_9 = ofArray([["className", "is-fullheight"], (elems_5 = [(elms_1 = singleton_1((elms = singleton_1(Navbar()), createElement("div", {
        className: "container",
        children: Interop_reactApi.Children.toArray(Array.from(elms)),
    }))), createElement("div", {
        className: "hero-head",
        children: Interop_reactApi.Children.toArray(Array.from(elms_1)),
    })), (props_2 = singleton_1((elems_2 = [Router()], ["children", Interop_reactApi.Children.toArray(Array.from(elems_2))])), createElement("div", createObj(Helpers_combineClasses("hero-body", props_2)))), (elms_2 = singleton_1((props_6 = ofArray([["className", "is-fluid"], (elems_3 = [createElement("div", createObj(Helpers_combineClasses("content", ofArray([["className", "has-text-centered"], ["children", "© 2022 General Digital Corporation"]]))))], ["children", Interop_reactApi.Children.toArray(Array.from(elems_3))])]), createElement("div", createObj(Helpers_combineClasses("container", props_6))))), createElement("div", {
        className: "hero-foot",
        children: Interop_reactApi.Children.toArray(Array.from(elms_2)),
    }))], ["children", Interop_reactApi.Children.toArray(Array.from(elems_5))])]);
    return createElement("section", createObj(Helpers_combineClasses("hero", props_9)));
}

export function App(pca) {
    const props = ofArray([["instance", pca], ["children", [PageContainer()]]]);
    return Interop_reactApi_1.createElement(msalProvider, createObj(props));
}

render(App(createClient(Msal_config)), document.getElementById("elmish-app"))

In order to get anything to work at all, I have to use the ofFunction function everywhere I use a ReactComponent, which I thought the attribute was supposed to do for me. Then, the generated code looks like this:

export function Router() {
    let elements;
    const patternInput = Feliz_React__React_useState_Static_1505(RouterModule_urlSegments(window.location.hash, 1));
    const updateUrl = patternInput[1];
    const currentUrl = patternInput[0];
    return RouterModule_router(createObj(ofArray([["onUrlChanged", updateUrl], (elements = toList(delay(() => {
        const matchValue = Url_Parse_1334CEF1(currentUrl);
        switch (matchValue.tag) {
            case 1: {
                return singleton(createElement("h1", {
                    children: ["Logging In..."],
                }));
            }
            case 0: {
                return singleton(react.createElement(Index, void 0));
            }
            case 2: {
                return singleton(react.createElement(Estimator, void 0));
            }
            case 3: {
                return singleton(react.createElement(TimeTracker, void 0));
            }
            default: {
                return singleton(createElement("h1", {
                    children: ["Error 404: Not Found"],
                }));
            }
        }
    })), ["application", react.createElement(react.Fragment, {}, ...elements)])])));
}

export function PageContainer() {
    let elems_5, elms_1, elms, props_6, elems_2, elms_2, props_10, elems_3;
    const props_13 = ofArray([["className", "is-fullheight"], (elems_5 = [(elms_1 = singleton_1((elms = singleton_1(react.createElement(Navbar, void 0)), createElement("div", {
        className: "container",
        children: Interop_reactApi.Children.toArray(Array.from(elms)),
    }))), createElement("div", {
        className: "hero-head",
        children: Interop_reactApi.Children.toArray(Array.from(elms_1)),
    })), (props_6 = singleton_1((elems_2 = [react.createElement(Router, void 0)], ["children", Interop_reactApi.Children.toArray(Array.from(elems_2))])), createElement("div", createObj(Helpers_combineClasses("hero-body", props_6)))), (elms_2 = singleton_1((props_10 = ofArray([["className", "is-fluid"], (elems_3 = [createElement("div", createObj(Helpers_combineClasses("content", ofArray([["className", "has-text-centered"], ["children", "© 2022 General Digital Corporation"]]))))], ["children", Interop_reactApi.Children.toArray(Array.from(elems_3))])]), createElement("div", createObj(Helpers_combineClasses("container", props_10))))), createElement("div", {
        className: "hero-foot",
        children: Interop_reactApi.Children.toArray(Array.from(elms_2)),
    }))], ["children", Interop_reactApi.Children.toArray(Array.from(elems_5))])]);
    return createElement("section", createObj(Helpers_combineClasses("hero", props_13)));
}

export function App(pca) {
    const props_2 = ofArray([["instance", pca], ["children", [react.createElement(PageContainer, void 0)]]]);
    return Interop_reactApi_1.createElement(msalProvider, createObj(props_2));
}

render(App(createClient(Msal_config)), document.getElementById("elmish-app"));

This works, but now I get a different set of warnings that I have no idea how to resolve:

Each child in a list should have a unique "key" prop.

I don't know how to add a key prop, because I need to return ReactElement and props are IReactProperty.

Also, as an aside, what are the benefits of moving to Fable 4 + Feliz 2 in terms of writing a React program?

sonicbhoc commented 1 year ago

Also, a major source of frustration with all of this is that I don't see any documentation explaining that Fable.React.Helpers.ofFunction needs to be called to make this work, if it is supposed to be required, or if any other functions would work. I literally don't know if it's intended behavior because I always have issues finding documentation for anything other than surface level usage of most F# libraries, one top of something that translates code to another language I'm only vaguely familiar with...

MangelMaxime commented 1 year ago

Hello @sonicbhoc, can you please provide the F# code so we can see more clearly what you are doing?

sonicbhoc commented 1 year ago

Oh, right. You might need to see that.

open Browser.Dom
open Fable
open Feliz
open Feliz.Router
open Feliz.React.Msal
open Feliz.Bulma
open Fable.React.Helpers

[<ReactComponent>]
let Router () =
    let (currentUrl, updateUrl) = React.useState (Router.currentUrl ())

    React.router [
        router.onUrlChanged updateUrl
        router.children [
            AuthenticatedTemplate.create [
                AuthenticatedTemplate.children [
                    match Url.Parse currentUrl with
                    | Url.LogIn -> Html.h1 "Logging In..."
                    | Url.Index -> ofFunction Index.Index () []
                    | Url.Estimator -> ofFunction Estimator.Estimator () []
                    | Url.TimeTracker -> ofFunction TimeTracker.User.View.TimeTracker () []
                    | Url.NotFound -> Html.h1 "Error 404: Not Found"
                ]
            ]
            UnauthenticatedTemplate.create [
                UnauthenticatedTemplate.children [
                    match Url.Parse currentUrl with
                    | Url.LogIn -> Html.h1 "Logging In..."
                    | Url.NotFound -> Html.h1 "Error 404: Not Found"
                    | _ -> ofFunction Index.Index () [ ]
                ]
            ]
        ]
    ]

[<ReactComponent>]
let PageContainer () =
    Bulma.hero [
        hero.isFullHeight
        prop.children [
            Bulma.heroHead [
                Bulma.container [
                    ofFunction Navbar.Navbar () []
                ]
            ]
            Bulma.heroBody [
                prop.children[
                    ofFunction Router () [] ]
            ]
            Bulma.heroFoot [
                Bulma.container [
                    container.isFluid
                    prop.children [
                        Bulma.content [
                            text.hasTextCentered
                            prop.text "© 2022 General Digital Corporation"
                        ]
                    ]
                ]
            ]
        ]
    ]

[<ReactComponent>]
let App pca =
    MsalProvider.create [
        MsalProvider.instance pca
        MsalProvider.children [
            ofFunction PageContainer () [ ]
        ]
    ]

ReactDOM.render (
    Msal.config |> createClient |> App,
    document.getElementById "elmish-app"
)
MangelMaxime commented 1 year ago

I am confident that you should not need the ofFunction for code like that.

Are you sure that all your components that call a hooks are decorated with [<ReactComponent>]?

It would be nice if you could remove your ofFunction one by one until you find the code that cause problem and provide us the full snippet.

If possible it would be even better if you can remove the code that doesn't cause the problem so we can focus on the problematic code with you.

Sorry to ask all that but that's to help you debug the issue and identify it so we can explain it or fix it if needed.

Zaid-Ajaj commented 1 year ago

Seeing PageContainer at call-site being PageContainer() instead of createElement(PageContainer, { }) strongly suggests that the attribute ReactComponent is not being picked up by Fable

You shouldn't need ofFunction that's legacy stuff

Can you share your package references here, better yet a repro with your project? Cause if ou are using a recent Feliz.CompilerPlugins v2.x and not the one that Feliz v1.68 depends on, then it won't work

sonicbhoc commented 1 year ago

My paket lockfile definitely pinned Feliz.CompilerPlugins 2.0. Thanks for the hint. I'll let you know if that fixes it.

sonicbhoc commented 1 year ago

That did the trick! I locked the version of Feliz.CompilerPlugins to ~> 1 in the paket.dependencies file and included it in my paket.references file. (That step is important; otherwise it still doesn't work.)

Thank you!

sonicbhoc commented 1 year ago

I closed the issue, but I just thought about something. The documentation should be updated to inform people that don't use the template to start a new project that they not only need to pull this library in as part of their paket dependencies, but also include them in the project, and ensure that the version of CompilerPlugins is ~> 1 for Feliz 1.68 and 2 for newer versions.

I would also love to see the package itself updated on nuget to depend on versions >1.10 and < 2.

panmona commented 1 year ago

I am running into the same issue with the ReactComponent attribute applied and a simple React.useEffect(fun () -> ()).

I have made a minimal Repro with only Feliz and the latest Fable: (@Zaid-Ajaj @MangelMaxime) https://github.com/panmona/fable3-esbuild/tree/repro-hooks-reactcomponent which uses the TabCounter example from the docs

To test it:

git clone git@github.com:panmona/fable3-esbuild.git
git checkout repro-hooks-reactcomponent
yarn
yarn start
# go to http://localhost:3000

I feel like this could have something to do with the changes to ReactComponent in #506 ? (@alfonsogarciacaro) In the compiled JavaScript there's just a normal function for this ReactComponent. So this somehow doesn't seem to correctly get picked up by Fable:

  // .build/App.js
  var import_react4 = __toESM(require_react());
  var import_client = __toESM(require_client());
  function TabCounter() {
    const patternInput = Feliz_React__React_useState_Static_1505(0);
    const setCount = patternInput[1];
    const count = patternInput[0] | 0;
    React_useEffect_3A5B6456(() => {
      document.title = toText(printf("Count = %d"))(count);
    });
    const children = ofArray([(0, import_react4.createElement)("h1", {
      children: [count]
    }), (0, import_react4.createElement)("button", {
      children: "Increment",
      onClick: (_arg1) => {
        setCount(count + 1);
      }
    })]);
    return (0, import_react4.createElement)("div", {
      children: Interop_reactApi.Children.toArray(Array.from(children))
    });
  }
  var root = (0, import_client.createRoot)(document.getElementById("elmish-app"));
  root.render(TabCounter());
MangelMaxime commented 1 year ago

@Zaid-Ajaj

When starting a new project from scratch requesting Feliz ~> 1, then paket/dotnet resolve Feliz.CompilerPlugins to 2.0.

The request made by @sonicbhoc is indeed important. Feliz 1 should request Feliz.CompilerPlugins below version 2.

The workaround is to manually restrict the version of Feliz.CompilerPlugins using nuget Feliz.CompilerPlugins ~> 1 for example.

MangelMaxime commented 1 year ago

Actually the same goes for Fable.React it should be restricted to Fable.React 8 because Fable.React 9 contains breaking changes that Feliz 1 doesn't support.

Zaid-Ajaj commented 1 year ago

@MangelMaxime Feliz doesn't depend on the main Fable.React library, only the types

panmona commented 1 year ago

Were you able to reproduce the behavior with the demo I linked above ( https://github.com/panmona/fable3-esbuild/tree/repro-hooks-reactcomponent ) and maybe have an idea what the underlying could be?

panmona commented 1 year ago

I found out that I didn't use the latest Fable Tool version. (theta-007 instead of theta-018) With theta-018 hooks work correctly.

So I think that this issue is now solved.