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

registerPage: No converter for component of type "function ConnectFunction(props) {... #82

Closed
3 tasks done
SantoJambit opened this issue Oct 21, 2019 · 6 comments
Closed
3 tasks done
Assignees
Labels
bug Something isn't working converter Concerns with a potential representation converter. core Concerns the piral-core library.

Comments

@SantoJambit
Copy link
Contributor

Bug Report

Prerequisites

  • Can you reproduce the problem in a MWE?
  • Are you running the latest version?
  • Did you perform a search in the issues?

Environment Details and Version

Piral 0.8.0

Description

calling registerPage with a component returned by react-redux's connect() method, I'm getting an exception:

No converter for component of type "function ConnectFunction(props) {...

Steps to Reproduce

  1. create a component using connect() of the react-redux package.
  2. Pass that component to app.registerPage()
  3. Notice it throwing an error.

Expected behavior

I expect this to work with react-redux components.

Actual behavior

An exception is thrown

Possible Origin

At this point, you are trying to use component.type:

const converter = converters[component.type];

But component.type is not a string, but instead is a function for this connected component.

@SantoJambit SantoJambit added the bug Something isn't working label Oct 21, 2019
@FlorianRappl
Copy link
Contributor

I'm confused. Every React component (I'm aware of) must be a function (either a class component or a FC).

Can you show the MWE? I tried, e.g., a class component and everything works without a flaw.

Just to be clear - if your code ends up in this part, you'd put in an object (as in typeof foo === 'object'). Glancing over react-redux I've also not seen an object returned, just (wrapped) functions. Maybe I'm missing something though.

@FlorianRappl FlorianRappl added the in-review The item is currently being reviewed. label Oct 22, 2019
@FlorianRappl FlorianRappl self-assigned this Oct 22, 2019
@FlorianRappl FlorianRappl added converter Concerns with a potential representation converter. core Concerns the piral-core library. labels Oct 22, 2019
@SantoJambit
Copy link
Contributor Author

@SantoJambit
Copy link
Contributor Author

I've taken a deeper look and I think the issue comes with React.memo. I've updated the MWE to use React.memo instead of react-redux to reproduce the issue.

@SantoJambit
Copy link
Contributor Author

Other react higher order components return objects as well: React.lazy, react.forwardRef.

The common part seems to be that an attribute $$typeof is present, with a value of Symbol(react.memo), Symbol(react.forward_ref), etc. respectively

@FlorianRappl
Copy link
Contributor

FlorianRappl commented Oct 22, 2019

I see, thanks for figuring out a way to reproduce this. I think we can deal with the $$typeof special prop for the moment.

According to the type definitions we have:

    interface ExoticComponent<P = {}> {
        /**
         * **NOTE**: Exotic components are not callable.
         */
        (props: P): (ReactElement|null);
        readonly $$typeof: symbol;
    }

So looking if $$typeof is there and a symbol could be sufficient.

Hotfix with 0.8.1 incoming later today / tomorrow morning.

Thanks for the report! 🍻

@FlorianRappl FlorianRappl added in-implementation The item is currently being implemented. and removed in-review The item is currently being reviewed. labels Oct 22, 2019
@FlorianRappl
Copy link
Contributor

Landed in develop.

@FlorianRappl FlorianRappl added in-testing The item is already out in preview and can be tested. and removed in-implementation The item is currently being implemented. labels Oct 22, 2019
@FlorianRappl FlorianRappl mentioned this issue Oct 22, 2019
8 tasks
@FlorianRappl FlorianRappl removed the in-testing The item is already out in preview and can be tested. label Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working converter Concerns with a potential representation converter. core Concerns the piral-core library.
Projects
None yet
Development

No branches or pull requests

2 participants