Doc on higher-order components #7869
Conversation
|
Can you add this to the nav_docs.yml under "advanced guides"? |
|
Yeah I just wanted to push this up before lunch |
|
G R E A T ! Still working on this, but it's getting close to being ready. The main example is based on Dan's blog post.I've avoided any discussion of mixins, since these are new docs and mixins are effectively deprecated. What do people think of this? Should we at least mention that they exist, even if we don't want people to use them?After this, I'll work on a companion doc about render callbacks that compares the two approaches.
File Changes
Patch Links:
|
| @@ -0,0 +1,338 @@ | |||
| # Higher-order components | |||
|
|
|||
| A higher-order component (HOC) is an advanced technique in React for reusing component logic. | |||
There was a problem hiding this comment.
I would stress that it is not a feature of React but a pattern right in the intro
|
|
||
| A higher-order component (HOC) is an advanced technique in React for reusing component logic. | ||
|
|
||
| Concretely, **a higher-order component is a function that returns a component.** An HOC accepts a component as one of its arguments, and returns an enhanced version of that component: |
There was a problem hiding this comment.
Maybe "that takes a component and returns a component"?
|
|
||
| ## Use HOCs for cross-cutting concerns | ||
|
|
||
| Components are the primary unit of code reuse in React. However, you'll find that some patterns aren't easily abstracted into traditional components. |
There was a problem hiding this comment.
This makes it sound like we claim render callbacks are "hard".
Maybe "sometimes it is harder to figure out how to abstract some pattern into a traditional component".
There was a problem hiding this comment.
Maybe instead of "hard" or "easy" we can say "straightforward"?
| render() { | ||
| return ( | ||
| <div> | ||
| {comments.map(function(comment) { |
There was a problem hiding this comment.
Let's just use arrows here, I plan to use them in such cases throughout the docs
|
|
||
| Because HOCs control the definition of the component class itself, they have the power to configure all aspects of a component's behavior, not just what goes in the `render` method. | ||
|
|
||
| Note: It is possible to use normal components in place of certain types of HOCs. See the doc on render callbacks to learn more about this technique and how it compares. |
There was a problem hiding this comment.
Let's avoid jargon like "doc"
| TODO | ||
|
|
||
| - Can't use HOCs inside render. | ||
| - Composing multiple HOCs together can be confusing, especially if you're not used to using function composition. |
There was a problem hiding this comment.
- instance methods don't work
acdlite
changed the title
| const EnhancedComponent = higherOrderComponent(WrappedComponent); | ||
| ``` | ||
|
|
||
| Whereas a component transforms props into UI, a higher-order component transforms one component into another component. |
There was a problem hiding this comment.
Getting nitpicky but I think this "component into a component" is pretty much repeated in the last two sentences as well. Can we just merge them into a single sentence?
There was a problem hiding this comment.
Maybe "transforms components themselves"?
|
|
||
| Components are the primary unit of code reuse in React. However, you'll find that some patterns aren't a straightforward fit for traditional components. | ||
|
|
||
| For example, say you have a CommentList component that subscribes to an external data source to render a list of comments: |
There was a problem hiding this comment.
Nit: put identifiers in backticks
| We'll create a function called `withSubscription` that works like this: | ||
|
|
||
| ```js | ||
| const CommentListWithSubscription = withSubscription( |
There was a problem hiding this comment.
I think it's very easy to not realize that these are "the same" React components. In other words, it's not obvious that you can "create" components by some other means than defining a class. It would be nice to stress something like "Instead of defining them explicitly, we will let a function called withSubscription() generate them."
|
|
||
| const BlogPostWithSubscription = withSubscription( | ||
| CommentList, | ||
| // An optional second argument passes the current props |
There was a problem hiding this comment.
This comment doesn't match the similar comment a few lines above which makes it confusing. Are these two examples subtle different?
I'm not sold on the need for inline comments here at all. Maybe better to explain that after code? I found our ```javascript{lineNo,lineStart-lineEnd,...} notation super helpful for highlighting specific snippets and drawing attention to them.
| }); | ||
| ``` | ||
|
|
||
| And here's what the implementation of `withSubscription` looks like: |
There was a problem hiding this comment.
Better to replace that sentence with a summary of why we are writing it?
|
|
||
| Because `withSubscription` is a normal function, you can add as many or as few arguments as you like. For example, you may want to make the name of the `data` prop configurable, to further isolate the HOC from the wrapped component. Or you could accept an argument that configures `shouldComponentUpdate`, or one that configures the data source. These are all possible because the HOC has full control over how the component is defined. | ||
|
|
||
| Note that like components, the contract between `withDataSubscription` and the wrapped component is entirely props-based. This makes it easy to swap one HOC for a different one, as long as they provide the same props to the wrapped component. An example of how this may be useful is if you change data-fetching libraries. |
There was a problem hiding this comment.
It was withSubscription in examples before?
There was a problem hiding this comment.
An example of how this may be useful is if you change data-fetching libraries.
=>
This may be useful if you change data-fetching libraries.
Less is more :-)
| function onlyUpdateForKeys(InputComponent, keys) { | ||
| // Mutates the input component's prototype | ||
| // *Don't do this!* | ||
| InputComponent.prototype.shouldComponentUpdate = function(nextProps) { |
There was a problem hiding this comment.
Can we use another example? AFAIK we want to treat sCU less strictly in the future and it would be nice if we didn't give an impression it can be legitimately used for e.g. blocking UI. Maybe data fetching would work better here?
There was a problem hiding this comment.
Hmm the problem with a data-fetching example is that you have to update render as well, which as I recall now was the reason I chose a shouldComponentUpdate example in the first place. Can you think of another example that doesn't involve render?
There was a problem hiding this comment.
Hmm not really. Maybe just leave it vague without going into details? Like use componentDidMount and leave /* ... */ there. Not sure if that's much better.
There was a problem hiding this comment.
Ooh, maybe a logging example a la react-lumberjack?
There was a problem hiding this comment.
This is a case non-mutating HOC can't handle :-(
There was a problem hiding this comment.
True but instead of state, though, it could log the latest props.
There was a problem hiding this comment.
Okay, let's use that example.
(Logging)
| const EnhancedComponent = onlyUpdateForKeys(InputComponent, ['foo', 'bar']) | ||
| ``` | ||
|
|
||
| There are a few problems with this. One is that the input component cannot be reused separately from the enhanced component. More crucially, if you apply another HOC to `EnhancedComponent` that *also* mutates `shouldComponentUpdate`, the first HOC's functionality will be overridden! Mutating HOCs are a leaky abstraction—the consumer must know how they are implemented in order to avoid conflicts with other HOCs. |
There was a problem hiding this comment.
It also doesn't work with functional components (if we care to mention that)
|
@gaearon Updated in response to your feedback, except for the |
| } | ||
| }); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Maybe add "Note that the returned component does not extend the wrapped component. This is not an example of inheritance. Instead, it uses the wrapped component in the render() method."
|
What about my earlier comments? |
|
@gaearon Which ones did I miss? |
|
Oh shoot there are a bunch I didn't see for some reason. Sorry! |
| const EnhancedComponent = higherOrderComponent(WrappedComponent); | ||
| ``` | ||
|
|
||
| Whereas a component transforms props into UI, a higher-order component transforms themselves. |
There was a problem hiding this comment.
I don't think it's quite correct to say a HOC transforms itself. How about:
"Whereas a component transforms props into UI, a higher-order component transforms a component into another component."
There was a problem hiding this comment.
Lol that's what I had before the last commit and @gaearon asked me to change it to this :D #7869 (comment)
There was a problem hiding this comment.
I kinda prefer it the old way, as well, but I believe Dan's point was that it sounded a bit repetitive.
| ```js | ||
| class CommentList extends React.Component { | ||
| constructor(props) { | ||
| super(); |
There was a problem hiding this comment.
I'll just remove the props argument entirely. Like context, it's not actually required.
| ```js | ||
| class BlogPost extends React.Component { | ||
| constructor(props) { | ||
| super(); |
| }} | ||
| ``` | ||
|
|
||
| `CommentList` and `BlogPost` aren't identical—they call different methods on `DataSource`, and they render different output. But much of their implementation is the same: |
|
|
||
| `CommentList` and `BlogPost` aren't identical—they call different methods on `DataSource`, and they render different output. But much of their implementation is the same: | ||
|
|
||
| - On mount, add a change listener to `DataSource`. |
There was a problem hiding this comment.
I really like this list and the following paragraph, I find it very educational
| const CommentWithRelay = Relay.createContainer(Comment, config); | ||
| ``` | ||
|
|
||
| By far, the most common signature for HOCs looks like this: |
There was a problem hiding this comment.
Is this actually the most common signature? I found this sentence confusing. How about just:
"Some HOCs, like React Redux's connect, look different:"
There was a problem hiding this comment.
I didn't do a proper, scientific survey or anything but in terms of usage I'm pretty sure it's true. I want to communicate that this signature is the de-facto standard. How could I make it less confusing?
There was a problem hiding this comment.
I also don't have a survey but mobx doesn't look like this for example - there you just stick @observer in front of your class rather than doing the double-calling thing. How about just not saying "by far" here.
There was a problem hiding this comment.
I feel like your @observer example proves my point. An advantage of the "double-calling thing" is that it's decorator compatible.
Maybe what you really want is a more in-depth discussion of decorators? I hesitate to do that until they're more stable. Babel doesn't even support them.
There was a problem hiding this comment.
I'm totally fine with getting rid of "by far" if that solves your concerns, though
| typeof EnhancedComponent.staticMethod === 'undefined' // true | ||
| ``` | ||
|
|
||
| To solve this, you could copy the methods onto the container before returning it |
| } | ||
| ``` | ||
|
|
||
| but this requires you to know exactly which methods need to be copied. You can use [hoist-react-non-statics](https://github.com/mridgway/hoist-non-react-statics) to automatically copy all non-React static methods: |
There was a problem hiding this comment.
no "but" at the beginning, capitalize This. Just for consistency with the phrasing around other inserted code blocks
acdlite
force-pushed
the
hoc-doc
branch
2 times, most recently
from
dc6d9c2 to
89727c5
Compare
| } | ||
| ``` | ||
|
|
||
| However, this requires you to know exactly which methods need to be copied. You can use [hoist-react-non-statics](https://github.com/mridgway/hoist-non-react-statics) to automatically copy all non-React static methods: |
There was a problem hiding this comment.
Typo: [hoist-react-non-statics] -> [hoist-non-react-statics]
|
Chatted with @acdlite & took another look, I think this is good to go. Thanks! |
|
😮 🎉 |
facebook-github-bot
added
the
React Core Team
Still working on this, but it's getting close to being ready. The main example is based on Dan's blog post.
I've avoided any discussion of mixins, since these are new docs and mixins are effectively deprecated. What do people think of this? Should we at least mention that they exist, even if we don't want people to use them?
After this, I'll work on a companion doc about render callbacks that compares the two approaches.