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

feat(*): Allow React functional components #331

Conversation

pavloko
Copy link
Contributor

@pavloko pavloko commented Oct 21, 2019

Rational

The release of Hooks allows to avoid classes completely, so it might be reasonable to allow functions to be used as top level components.

Changes

I've added a check to make sure cmp is a function and since Angular's directives and components are passed as strings it should be safe to attribute that component to React.

React does similar check internally and ui-router's heuristic with these changes included would be similar.

@christopherthielen I'd appreciate your feedback on this.

Angular component or directives are always passed as strings,
so checking if the passed component is a function should be
sufficient to attribute the component to React.
@pahen
Copy link
Contributor

pahen commented Oct 23, 2019

This would be awesome to support.

@christopherthielen
Copy link
Member

christopherthielen commented Oct 26, 2019

since Angular's directives and components are passed as strings it should be safe to attribute that component to React.

👍 I'm honestly surprised this hasn't come up before! I thought we already supported functional components. Thanks a bunch 😄

I believe that in my codebase I'm using component: FnComponent, $type: 'react'

@christopherthielen christopherthielen merged commit b6a296d into ui-router:master Oct 26, 2019
@pahen
Copy link
Contributor

pahen commented Oct 28, 2019

This works for functional React components but it breaks if the component is using React.memo since it will be an object then and not a function. Using $type: 'react' works though 👍

@pavloko pavloko deleted the pavloko/allow-functional-components branch October 28, 2019 11:14
@pavloko
Copy link
Contributor Author

pavloko commented Oct 28, 2019

@pahen Do you think ui-router should handle it somehow?

@pahen
Copy link
Contributor

pahen commented Oct 28, 2019

Yes, since using React.memo in functional components is very common and it's included in React. So I think it would be very nice if ui-router could support it out of the box @pavloko

@pavloko
Copy link
Contributor Author

pavloko commented Oct 28, 2019

Sounds good! I'll try to experiment with it sometime in the next 2 weeks.

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

Successfully merging this pull request may close these issues.

None yet

3 participants