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

Update from v5 to v6 breaks application #194

Closed
kerams opened this issue May 2, 2020 · 7 comments
Closed

Update from v5 to v6 breaks application #194

kerams opened this issue May 2, 2020 · 7 comments

Comments

@kerams
Copy link
Contributor

kerams commented May 2, 2020

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
Copy link

jonasdw commented May 4, 2020

@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
Copy link
Contributor Author

kerams commented May 4, 2020

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
Copy link
Member

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

@alfonsogarciacaro
Copy link
Member

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:

  • Discard the cache. But then we'll have again the issue with components recreated multiple times (generic values, nested functions) mentioned in the description of New API for FunctionComponent #188.
  • Use the type API proposed originally in New API for FunctionComponent #188 that nobody likes :)
  • Deprecate FunctionComponent.Of and offer two alternatives with better semantics FunctionComponent.Cached/NonCached.

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

@MangelMaxime
Copy link
Member

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
Copy link
Member

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

@alfonsogarciacaro
Copy link
Member

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.

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

No branches or pull requests

5 participants