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

Warn about deprecated SVG attributes only once #6184

Merged
merged 1 commit into from
Mar 4, 2016
Merged

Warn about deprecated SVG attributes only once #6184

merged 1 commit into from
Mar 4, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 4, 2016

This fixes a missing assignment in #5714 that caused tons of extraneous warnings.

Before

screen shot 2016-03-04 at 15 59 58

## After

screen shot 2016-03-04 at 16 08 03

Reviewers: @zpao

@@ -33,6 +33,7 @@ if (__DEV__) {
warnedSVGAttributes.hasOwnProperty(name) && warnedSVGAttributes[name]) {
return;
}
warnedSVGAttributes[name] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a deal breaker, but seems a bit weird that warnedSVGAttributes gets set to true even if a warning never fired (ie. bailed out in the next three lines). Maybe a few of these lines can be re-arranged to avoid this, maybe not worth the headache.

@jimfb
Copy link
Contributor

jimfb commented Mar 4, 2016

Two nits so small I wasn't even sure if I'd bother mentioning them, feel free to ignore if you want.

This fixes a missing check in #5714
@facebook-github-bot
Copy link

@gaearon updated the pull request.

@gaearon
Copy link
Collaborator Author

gaearon commented Mar 4, 2016

I made two changes:

  • Expanded the test to make the intent more clear
  • Grouped the checks by their purpose with newlines

@jimfb
Copy link
Contributor

jimfb commented Mar 4, 2016

👍

gaearon added a commit that referenced this pull request Mar 4, 2016
Warn about deprecated SVG attributes only once
@gaearon gaearon merged commit 36798f7 into facebook:master Mar 4, 2016
@gaearon gaearon deleted the fix-svg-warning branch March 4, 2016 18:14
zpao added a commit to zpao/react that referenced this pull request Mar 9, 2016
zpao added a commit to zpao/react that referenced this pull request Mar 11, 2016
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.

3 participants