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

Wrapper Components - reconciler support #167

Closed
wants to merge 1 commit into from

Conversation

mbest
Copy link

@mbest mbest commented May 31, 2020

From facebook/react#18993

In our project, we often create components whose purpose is to constrain or enhance another component. We've run into a problem with this approach when a component needs to change how it's displayed based on some state.

For example, suppose we have an IconButton for a button that just contains an icon, and an ActionButton for one that shows text and an icon, and we want to switch a button between these two based on some state. Even though they both render a Button, React sees these as different components and re-creates the button itself when changing between them. If the button was in focus, it will lose focus from this state change.

Rendered View

@mbest
Copy link
Author

mbest commented Jun 11, 2020

Would love to get some feedback. @bvaughn?

@gaearon
Copy link
Member

gaearon commented Aug 19, 2021

Sorry for a long response time, see #182.

The main problem is that a component "claiming" to be compatible with another component doesn't mean that they're actually compatible. In particular, their list of Hooks will likely be different. So it's not safe to "morph" them like this and "reuse" them, assuming that you want the state/refs to be reused. Maybe you don't want state to be reused, but still want the DOM to be preserved? But this is of very limited usefulness, since it would break the moment their root element types stop matching. It also breaks encapsulation a bit because once I "lock in" what a component "renders to", it's harder to change. I don't know what code depends on that semantically since I'd have to check every callsite and see what other components could be rendered in the same spot (and thus be "compatible").

So this seems like a complex solution adding many edge cases to something that already has an idiomatic solution. When you want reuse, make it a single component. This makes it explicit that you want it to be reused — even if it might make for a slightly more awkward API. And when you don't want reuse, make it two different components. Your example with a button demonstrates that it might make sense (in that particular case) to actually keep it as a single component. Or maybe it's more of a focus management question (and an exploration like #104 could consider what should happen when a component is removed).

Note also that you don't always have to create new components. E.g. instead of having ActionButton and IconButton, you could do <Button><Icon /></Button> and <Button><Icon /> Some Text</Button>, and leave it at that. I'd actually suggest this instead of making a high-level component for every possible variation. Hope this makes sense!

@gaearon gaearon closed this Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants