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

Make component stack last argument for deprecation warnings #16384

Merged
merged 1 commit into from
Aug 13, 2019

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 13, 2019

Component stack is the last argument when we use warning().

There are a few cases where we append stack manually. (Maybe some of them don't need to.) Those were originally written to be more readable. However, increasingly we're using tooling that treats last argument specially if it looks like a component stack. Both in RN and in FB5. We'll likely do the same in CRA/Next/etc.

So let's always put component stack last consistently. This makes it easier to build richer UIs and looks decent in console anyway.

Screen Shot 2019-08-13 at 10 57 24 PM

@sizebot
Copy link

sizebot commented Aug 13, 2019

Warnings
⚠️

Base commit is broken: 21d6395

Generated by 🚫 dangerJS

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes some of the messages read a little more awkwardly, but I share your concern about tooling treating the last argument as special. Seems like a reasonable trade off.

How do we enforce this for future warnings?

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 13, 2019

We should probably just migrate the rest to warning().

@gaearon gaearon merged commit e0a521b into facebook:master Aug 13, 2019
@gaearon gaearon deleted the warnn branch August 13, 2019 22:25
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.

5 participants