Added warning to <Context.Provider> in case no value prop is provided #19054

charlie1404 · 2020-05-30T17:34:59Z

Summary

This PR aims to add a fix for #19020

There is a duplication in what I have done in files
- packages/react-reconciler/src/ReactFiberBeginWork.new.js
- packages/react-reconciler/src/ReactFiberBeginWork.old.js

In feature flag saw const enableNewReconciler = false but still cannot jump to any solid conclusions hence added in both.

Test Added
packages/react-reconciler/src/__tests__/ReactNewContext-test.js test to detect warning.

Tests Updated Reason
As now <Context.Provider> now throws a warning if value prop is not given, and hence updated the ones which were not having value prop to have null now.

Points to be discussed

What should be the warning message, (not really good with this) 😅
Should this be included in both old and new, or I have added in the totally wrong place.
- if correct, out of curiosity why there are 2 files, seems almost identical, and only these but a whole lot files are in the same format. (please ignore if very long explanation and out of the scope of this PR)
Should any more test cases to be added in this?

cc @brunogonzales @heath-freenome

codesandbox · 2020-05-30T17:37:08Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1603910:

Sandbox	Source
cool-leftpad-pem6m	Configuration

sizebot · 2020-05-30T17:41:15Z

	Warnings
⚠️	Could not find build artifacts for base commit: `1d85bb3`

Size changes (stable)

Generated by 🚫 dangerJS against 1603910

sizebot · 2020-05-30T17:42:36Z

	Warnings
⚠️	Could not find build artifacts for base commit: `1d85bb3`

Size changes (experimental)

Generated by 🚫 dangerJS against 1603910

charlie1404 · 2020-05-30T18:34:13Z

did like the name of the code sandbox 🤣.
Edit:
code sandbox bot edited the old message

old	new
cocky-margulis-zss43	unruffled-bardeen-vj2s6

wondering why cannot just update deps it old one 😞

heath-freenome · 2020-06-03T20:29:08Z

Question, would making the value prop .isRequired also accomplish this?

charlie1404 · 2020-06-04T09:41:23Z

Question, would making the value prop .isRequired also accomplish this?

@heath-freenome
Initially gave a thought on that idea, but did not proceed for 2 reasons:

If we use prop types there is a possibility that value prop can actually be null or undefined, say if the value is being set from some key in state, which is an object, but the key used is null or undefined for some reason, in that case, it will start adding warnings to console, not sure if we should do that, worth discussing. The 'value' in props only gives a warning if there is no value prop supplied to <Context.Provider>.
Now if we use prop types I had to do Object.assign with workInProgress.type.propTypes, but as this is an internal component and had no prop types defined hence was undefined, not sure if I should add a completely new object, ignoring old one or still do Object.assign maybe I was looking at the wrong place.

But pointing to prop types gave me a better message, thanks for that, updating it now.

eps1lon · 2020-06-04T09:50:14Z

React just recently removed all runtime dependencies to prop-types. It's unlikely they re-introduce it. Especially given prop-types cannot distinguish between a missing prop and an undefined prop which @charlie1404 already pointed out.

gaearon · 2020-06-30T16:48:24Z

packages/react-reconciler/src/ReactFiberBeginWork.new.js

+      if (!hasWarnedAboutUsingNoValuePropOnContextProvider) {
+        hasWarnedAboutUsingNoValuePropOnContextProvider = true;
+        console.error(
+          'The prop `value` is required in `Context.Provider`, have you misspelled it',


Let's reword this a bit:

The `value` prop is required for the `<Context.Provider>`. Did you misspell it or forget to pass it?

Thanks for the message 😅 .

gaearon · 2020-06-30T16:48:37Z

packages/react-reconciler/src/ReactFiberBeginWork.old.js

+      if (!hasWarnedAboutUsingNoValuePropOnContextProvider) {
+        hasWarnedAboutUsingNoValuePropOnContextProvider = true;
+        console.error(
+          'The prop `value` is required in `Context.Provider`, have you misspelled it',


gaearon · 2020-06-30T16:49:24Z

packages/react/src/__tests__/ReactContextValidator-test.js

@@ -263,13 +263,13 @@ describe('ReactContextValidator', () => {

    class Component extends React.Component {
      render() {
-        return <TestContext.Provider />;
+        return <TestContext.Provider value={null} />;


Make this value={undefined} instead and you won't need to change the message.

now using undefined and reverted message

gaearon

Small nits and this is good to go.

gaearon · 2020-06-30T18:50:59Z

Coolio, thanks.

added warning to context.provider in case no value prop

Verified

This commit was signed with a verified signature.

charlie1404 Shailendra Gupta

GPG key ID: A193985FFE83D98B Learn about signing commits

Loading status checks…

f1ec9b7

facebook-github-bot added the CLA Signed label May 30, 2020

update message

Verified

This commit was signed with a verified signature.

charlie1404 Shailendra Gupta

GPG key ID: A193985FFE83D98B Learn about signing commits

Loading status checks…

8c85dc0

gaearon reviewed Jun 30, 2020

View changes

gaearon requested changes Jun 30, 2020

View changes

updated message and pass undefined

Verified

This commit was signed with a verified signature.

charlie1404 Shailendra Gupta

GPG key ID: A193985FFE83D98B Learn about signing commits

Loading status checks…

1603910

gaearon approved these changes Jun 30, 2020

View changes

charlie1404 deleted the charlie1404:19020-warn-if-no-value-prop-in-context-provider branch Jul 1, 2020

eps1lon mentioned this pull request Aug 21, 2020

Feature request: have Context.Provider throw error if missing `value` prop #19020

Closed

facebook / react

Added warning to <Context.Provider> in case no value prop is provided #19054

Added warning to <Context.Provider> in case no value prop is provided #19054

charlie1404 commented May 30, 2020 •

edited

codesandbox bot commented May 30, 2020 •

edited

sizebot commented May 30, 2020 •

edited

sizebot commented May 30, 2020 •

edited

charlie1404 commented May 30, 2020 •

edited

heath-freenome commented Jun 3, 2020

charlie1404 commented Jun 4, 2020

eps1lon commented Jun 4, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gaearon left a comment

gaearon commented Jun 30, 2020

facebook / react

Join GitHub today

GitHub is where the world builds software

Added warning to <Context.Provider> in case no value prop is provided #19054

Added warning to <Context.Provider> in case no value prop is provided #19054

Conversation

charlie1404 commented May 30, 2020 • edited

Summary

codesandbox bot commented May 30, 2020 • edited

sizebot commented May 30, 2020 • edited

Size changes (stable)

sizebot commented May 30, 2020 • edited

Size changes (experimental)

charlie1404 commented May 30, 2020 • edited

heath-freenome commented Jun 3, 2020

charlie1404 commented Jun 4, 2020

eps1lon commented Jun 4, 2020

This comment has been minimized.

gaearon Jun 30, 2020 Member

This comment has been minimized.

charlie1404 Jun 30, 2020 Author Contributor

This comment has been minimized.

charlie1404 Jun 30, 2020 Author Contributor

This comment has been minimized.

gaearon Jun 30, 2020 Member

This comment has been minimized.

gaearon Jun 30, 2020 Member

This comment has been minimized.

charlie1404 Jun 30, 2020 Author Contributor

gaearon left a comment

gaearon commented Jun 30, 2020

Essential cookies

Always active

Analytics cookies

charlie1404 commented May 30, 2020 •

edited

codesandbox bot commented May 30, 2020 •

edited

sizebot commented May 30, 2020 •

edited

sizebot commented May 30, 2020 •

edited

charlie1404 commented May 30, 2020 •

edited

gaearon Jun 30, 2020
Member

charlie1404 Jun 30, 2020
Author Contributor

charlie1404 Jun 30, 2020
Author Contributor

gaearon Jun 30, 2020
Member

gaearon Jun 30, 2020
Member

charlie1404 Jun 30, 2020
Author Contributor