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

[Fix] no-unknown-property: No unknown property doesn't recognize svg properties #3381

Merged
merged 1 commit into from Sep 3, 2022
Merged

[Fix] no-unknown-property: No unknown property doesn't recognize svg properties #3381

merged 1 commit into from Sep 3, 2022

Conversation

AhmadMayo
Copy link
Contributor

fixes #3380

I don't know if the added attributes are exhaustive or not. I sure hope MDN covered all possible attributes.

@codecov
Copy link

codecov bot commented Sep 3, 2022

Codecov Report

Merging #3381 (33c27c2) into master (512909b) will not change coverage.
The diff coverage is n/a.

❗ Current head 33c27c2 differs from pull request most recent head 22cedd8. Consider uploading reports for the commit 22cedd8 to get more accurate results

@@           Coverage Diff           @@
##           master    #3381   +/-   ##
=======================================
  Coverage   97.57%   97.57%           
=======================================
  Files         123      123           
  Lines        8954     8954           
  Branches     3268     3268           
=======================================
  Hits         8737     8737           
  Misses        217      217           
Impacted Files Coverage Δ
lib/rules/no-unknown-property.js 97.56% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Looks great!

Comment on lines +32 to +34
fill: ['svg'],
property: ['meta'],
viewBox: ['svg'],
Copy link
Member

Choose a reason for hiding this comment

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

i added these as well.

@ljharb ljharb merged commit 22cedd8 into jsx-eslint:master Sep 3, 2022
@AhmadMayo AhmadMayo deleted the no-unknown-property-recognize-svg-properties branch September 3, 2022 09:34
@@ -29,6 +29,9 @@ const DOM_ATTRIBUTE_NAMES = {
const ATTRIBUTE_TAGS_MAP = {
// image is required for SVG support, all other tags are HTML.
crossOrigin: ['script', 'img', 'video', 'audio', 'link', 'image'],
fill: ['svg'],

Choose a reason for hiding this comment

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

fill appears to be OK on a number of other tags, such as path

Copy link
Member

Choose a reason for hiding this comment

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

This should be handled by #3385

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

react/no-unknown-property throws on valid properties
3 participants