-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Offer an opinionated helper for HOC creation for authors #9345
Comments
Hey @Pomax, for taking the time to write this up!
There is a doc page on higher-order components in the official React docs. Do you have any suggestions on what we could add to make it clearer or more robust? Better documentation and discussions focusing on best practices might be better than expanding the public API.
Isn't a HOC typically a function that takes a single component class and returns another composing it? Can you expand on HOCs where multiple classes are passed in? Are you referring to utilities like I think offering a class-based HOC solution would probably be moving in the wrong direction, as existing HOCs are typically just functions and are composed using functional patterns.
Standardizing access to elements by recommended a @acdlite you've worked with HOCs extensively with |
Mostly I'm trying to think of a good solution where the HOC functions all return an object with same API for accessing the "actual content wrapped" even if they share nothing else. I will admit: I've not seen any HOCs that take more than one component class as input in the wild, but I'm sure I've not seem most of them, and there is technically nothing in the docs or the code that prevent you from writing a HOC like this:
I admit, writing that feels super weird, but React will happily run with it if you write your HOC that way. If HOCs "should" only wrap a single component then that could be a thing to point out over on https://facebook.github.io/react/docs/higher-order-components.html, but the main thing I'm looking for is the docs being an authority and saying:
which in itself is of course only half a step away from then following up with:
(those might be misnomers, I guess technically those would be React.HOCClass and React.createHOCClass but I will the first to admit those are super inconvenient to read and write!) For purely functional HOCs, you're right in that this solution falls short - I will have to think about that aspect a bit harder (my own HOC composes a full class as it needs to hook into lifecycle methods itself, so that made me come at this from a fully qualified Component perspective). I'd love to hear thoughts on how to ensure that there is a way to get to the original Component's class/element in a HOC chain both for fully qualified component HOCs and purely functional HOCs though. It's a problem that seems to come up time and again but is never quite big enough to boil over into a "we really need to fix this" so everyone's rolling their own solutions and we can probably come up with something better =) |
That is my original problem described in the linked comment in the thread starter post. There is no standard way for A and B geting access to C's ref in
Agree with you, that it's moving things in the other direction than they are heading nowadays. From my perspective there is not much need for new APIs, more just the best practices documented. Kinda protocol that might be applied to make HOCs interoperable.
Standard name doesnt have to contain string value. It would work with functions as well, but as HOC doesnt know when to stop its not possible for it not to leak a 'special' |
Coming back to this, I don't think the solution proposed here of expanding the API to provide first-class HOC support is something that will be pursued. The core of the issue is that it's difficult to access refs of wrapped components. This issue is being worked on independently (see #10581, #4213 for example) and a solution to that problem will likely address the bulk of this one. |
HOCs are great, I think we can all agree. What is not so great, however, is that React has no opinion on how people should implement HOCs, and so both users and authors alike have no guideline when it comes to implementing a consistent api for "using multiple HOCs", especially when those HOCs get chained. As such, I think that there's an opportunity to make the lives of both developers who work with React, as well as HOC maintainers who have to make sure their HOCs work in a variety of code bases easier by having React be slightly opinionated on how to write a HOC.
To describe the problem: right now there is no established way to ask a HOC for the component class(es) that were put in, nor an established way to access the "real element" that a HOC wraps. As such, each HOC has to either settle for "too bad for my users" or invent its own API for this, and as many have gone the second route, there are a fair few different ways that this access has been implemented over different HOCs.
To fix this, I'd like to suggest establishing a best practice, guided by React, by adding a
React.createHOC({ ... })
mechanism with correspondingimport HOC from React; class Thing extends HOC { ... }
" to the next version of React, so that users and authors no longer need to guess APIs.My suggestion would be for this React.HOC class to define:
ref
based on the Component class names)As an example (and the following code is public domain - which I unfortunately need to be point out because I am unable to sign the CLA necessary to file PRs, so I can't file a PR in conjunction to this issue. As such, the following code lacks as much implementation as possible so as to allow for PRs by others):
This lets React solve the problem of standardizing the barebones functionality that users and authors should be able to rely on to "just work", without interfering with how HOCs currently work, or dictating how people should write their HOCs: you can keep writing HOCs as plain Components that wrap one or more components, but extending from HOC now offers you a zero-effort option to impart some functionality for free that users will appreciate.
There is an interesting suggestion in jsx-eslint/eslint-plugin-react#678 (comment) that instead relies on rebinding
props
, but this might not good enough as this would also expose refs used by the HOC for internal purposes, and the user should not be able to fiddle with.Anyway, this is mostly an issue filed to get a ball rolling on not so much standardizing "how HOCs should be made" (it feels like React should stay unopinionated on that) but on having React offer a helping opinion on one way in which you can write a HOC, with the promise that using that means you're now compatible with any other HOC that uses the same suggestion.
The text was updated successfully, but these errors were encountered: