Skip to content
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

feat(createContext): Include displayName in warning #18386

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Mar 25, 2020

Summary

Seems like this is not only used internally at facebook. I included the displayName that should be set when warning to make it more obvious where the warning is coming from.

This also explicitly documents the strategy by which this warning is deduped.

Test Plan

react.development.js:319 Warning: Setting `displayName` on Context.Consumer has no effect. You should set it directly on the context with Context.displayName = 'Location.Consumer'.
printWarning @ react.development.js:319
react.development.js:319 Warning: Setting `displayName` on Context.Consumer has no effect. You should set it directly on the context with Context.displayName = 'Route.Consumer'.

Looks more useful to me.

@sizebot
Copy link

sizebot commented Mar 25, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against b2f5d37

@sizebot
Copy link

sizebot commented Mar 25, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against b2f5d37

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2020

Seems like this is not only used internally at facebook.

I don't know what you mean by this. Yes, it's a public warning. It concretely says that doing this is unsupported and you should assign to the context itself instead.

@gaearon gaearon merged commit 7516bdf into facebook:master Apr 1, 2020
@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2020

Makes sense.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Apr 1, 2020

It concretely says that doing this is unsupported and you should assign to the context itself instead.

It was just in reference to the original PR that had to be reverted because it broke tests at facebook internally (#18223 (comment)). I thought it wasn't used by others because it didn't have any effect.

@eps1lon eps1lon deleted the fix/context-consumer-set-warning branch April 1, 2020 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants