fable-compiler / fable-react

Fable bindings and helpers for React and React Native
MIT License
273 stars 67 forks source link

Update from v5 to v6 breaks application #194

Closed kerams closed 1 year ago

kerams commented 4 years ago

I've updated my application to v6 of Fable.React and while everything compiles fine, I am now getting a blank page with this error:

Uncaught TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator))
    at eval (webpack-internal:///./.fable/fable-library.2.8.4/Seq.js:692)
    at Object.eval (webpack-internal:///./.fable/fable-library.2.8.4/Seq.js:385)
    at Object.eval (webpack-internal:///./.fable/fable-library.2.8.4/Seq.js:385)
    at fold (webpack-internal:///./.fable/fable-library.2.8.4/Seq.js:551)
    at ofSeq (webpack-internal:///./.fable/fable-library.2.8.4/List.js:277)
    at attributeTags (webpack-internal:///./src/Client/Common.fs:530)
    at prompt (webpack-internal:///./src/Client/Common.fs:538)
    at renderWithHooks (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:14803)
    at mountIndeterminateComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:17482)
    at beginWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:18596)

prompt is a function component defined in a shared module like this:

let memo name r = FunctionComponent.Of (memoizeWith = equalsButFunctions, displayName = name, render = r)

let prompt = memo "prompt" (fun (x: {| some props |}) ->
    Columns.columns [] [
        attributeTags blah
    ])

and used like div [] [ prompt {| prop = "" |} ]. It's worth mentioning though that it is not rendered anywhere near the entry point of the application, so I don't know why the inside of its render function should be called and throw errors on page load.

Reverting back to v5 (and an earlier version of Feliz, the last one that depends on v5, and which I don't use, but is a transitive dependency of Feliz.Router) causes everything to go back to normal.

jonasdw commented 4 years ago

@kerams I recently had the same issue after upgrading to version 6.

I had a brief chat with @Zaid-Ajaj and he told me that "the internal implementation of FunctionComponent.Of was updated, so the definition is cached"

If u use the new implementation or alternatively use Feliz.React.functionComponent which is not effected by the old implementation you should be able to fix your problem.

kerams commented 4 years ago

The API in the first post of the pull request does not exist. What I am already using still appears to be the only way to create a function component.

I've swapped

let memo name r = FunctionComponent.Of (memoizeWith = equalsButFunctions, displayName = name, render = r)

for

let memo name (r: _ -> ReactElement) = Feliz.React.memo (name, equalsButFunctions, r)

and it did actually help. Still, it would be nice to know why the original approach is no longer valid (ideally all this should also be mentioned in the release notes or a migration guide).

It's also worth mentioning that all components are now called Anonymous (Feliz's or Fable.React's fault?)

Thanks, @jonasdw.

Zaid-Ajaj commented 4 years ago

@kerams The name of the components isn't showing when you provide the name parameter? if so, can you file an issue on Feliz :pray:

alfonsogarciacaro commented 4 years ago

Hmm, I forgot to take into account the cases where FunctionComponent.Of is called from a helper and not directly assigned to a value šŸ¤¦ The API proposed originally in #188 was meant to avoid this but we finally used a cache to maintain the same API and avoid breaking changes (we got breaking behavior instead!).

In your case @kerams, the way to fix the issue would likely be to remove the memo helper (although I don't understand why the render function is called before the component is rendered):

let prompt =
 FunctionComponent.Of (memoizeWith = equalsButFunctions,
  render = fun (x: {| some props |}) ->
    Columns.columns [] [
        attributeTags blah
    ])

More generally, I'm not sure how we can avoid issues like this in the future:

Any thoughts? @Zaid-Ajaj @MangelMaxime @Luiz-Monad

MangelMaxime commented 4 years ago

If the type API prevents non-supported usage of the FunctionComponent I am ok with it personally.

My concerns with better semantics are that you need to understand the difference between both of then and TBH I don't think this is that easy to do.

I am in favour of a more opinionated API

alfonsogarciacaro commented 4 years ago

Then go back to the original proposal of #188, will @Zaid-Ajaj be OK with that? ;)

alfonsogarciacaro commented 4 years ago

Ok, I'm giving it a new try at fixing this (better said, at giving a better error message to users) in #195, please have a look if you have a moment.