-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Document class contextType as the primary consuming mechanism #1265
Conversation
Deploy preview for reactjs ready! Built with commit c9fe3768478d1533970ed73d1b62ce7438a99403 |
Deploy preview for reactjs ready! Built with commit d7b724e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good overall. Left some thoughts.
|
||
### `Consumer` | ||
All consumers that are descendants of a Provider will re-render whenever the Provider's `value` prop changes. The propagation from Provider to its descendant consumers is not subject to the `shouldComponentUpdate` method, so the consumer is updated even when an ancestor component bails out of the update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should clarify that context is subject to a component's own shouldComponentUpdate
method, just not its ancestors. (Seems like this wording could be misleading.)
render() { | ||
let value = this.context; | ||
/* render soemthing based on the value of MyContext */ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth showing shouldComponentUpdate
here so we can illustrate this.context
vs the nextContext
param? Or would that muddy the waters too much?
If not here, we should mention that param/behavior somewhere now that the "Accessing Context in Lifecycle Methods" section is gone.
} | ||
render() { | ||
let value = this.context; | ||
/* render soemthing based on the value of MyContext */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* render soemthing based on the value of MyContext */ | |
/* render something based on the value of MyContext */ |
I think the HOC examples should be kept around. I'm not sure that consuming just a single context from a component is actually that common. The new API works well as a 1-1 replacement for some usages of legacy context, but most apps I've seen use a combination of HOCs and render props to inject data providers and there are usually more than one (for example react-intl |
This looks great overall!
I'm not sure this case is so edge, it was the first question that came to my mind when I knew about this
Is it supported to access multiple contexts from this.context? If so, yes please add info about this somewhere from the beginning. I imagine something like this:
or
EDIT: Just noticed this was already requested here: facebook/react#13728 (comment) |
You can still use render prop API for cases when you need that. This just attempts to cover the 80% case. |
facebook/react#13728
This documents the Class.contextType API.
The intention here is to document this as the primary way to consume a Context from a class, and since classes are still the mainstream component API, it's the primary way to consume a Context period.
The Consumer API is documented as the way to consume a Context from a function component.
As a result I could remove the following examples:
Accessing Context in Lifecycle Methods - Accessing context in life-cycles is now easy in since the primary way makes that the default. Technically this is still relevant for multiple context in a single class, but it's such an edge case that I feel like it belongs on Stack Overflow or some recipes section. Not the primary documentation.
Forwarding Refs to Context Consumers - While this is still something you might want to do, this is not different than forwarding refs in general. It used to be that this pattern was unique in that when you converted from a class to use Consumer + forwardRef, you would get into this awkward situation. However, if the primary API is to use
.contextType
on a class, then this situation doesn't happen so doesn't seem to add the confusion.Consuming Context with a HOC - This pattern is possibly still relevant but it's only relevant because the render prop syntax is kind of a mess when you compose many of them. However, it looks like we're not going to be recommending this pattern and instead look to make it easier to consume context in other ways like the unstable_read proposal. Since this pattern isn't critical even before that's stable I think we should just remove it.