-
Notifications
You must be signed in to change notification settings - Fork 46.8k
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
Warn when second argument is passed to useContext #14729
Conversation
@@ -613,8 +613,114 @@ describe('ReactDOMServerHooks', () => { | |||
}); | |||
|
|||
describe('useContext', () => { | |||
itThrowsWhenRendering( |
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.
The diff confused git. This is not a new test.
All I did is move the useContext()
test with bits into readContext()
section below.
}, | ||
); | ||
|
||
itRenders('warns when bitmask is passed to useContext', async render => { |
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.
This is a new test.
itRenders( | ||
'can use the same context multiple times in the same function', | ||
'can read the same context multiple times in the same function', |
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.
This is existing test which I made use readContext
instead.
@@ -1300,81 +1324,7 @@ describe('ReactNewContext', () => { | |||
}); | |||
|
|||
describe('readContext', () => { | |||
// Context consumer bails out on propagating "deep" updates when `value` hasn't changed. |
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.
This test just moved below in the diff.
) { | ||
const dispatcher = resolveDispatcher(); | ||
if (__DEV__) { | ||
warning( |
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.
This is the actual change.
React: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: ba6477a...bd87a94 react
Generated by 🚫 dangerJS |
Did you meant |
@gaearon Does this make the |
Yes there’s always a chance of unstable APIs getting removed. It doesn’t make it more unstable than the Consumer prop version. It’s just easier to pass by accident and get weird results. Which is why this one warns. |
Got it, thanks for the clarification! |
@gaearon So I have another question -- is this meant to stop anyone from using |
The thing I see is that with this flag However I can't seem to get it working without a
This is the way i use it,
Please let me know if i'm doing anything wrong. Also is this |
This leaves us the ability to later change its meaning. It also warns early avoids confusing cases like
[].map(useContext)
which accidentally suppliesobservedBits
(#14708 (comment)).Internal
readContext
on the dispatcher doesn't have the warning.