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

[New] Introduce a plugin-wide setting for custom components. #844

Merged
merged 1 commit into from Apr 8, 2022

Conversation

pcorpet
Copy link
Contributor

@pcorpet pcorpet commented Mar 24, 2022

For #174

This is an early version of the change I want to introduce: there are still some rules that I haven't updated.
Can you start a review on the API and the implementation to see if I should finish it please?

@codecov
Copy link

codecov bot commented Mar 24, 2022

Codecov Report

Merging #844 (64dcac6) into main (1826628) will increase coverage by 0.03%.
The diff coverage is 99.76%.

@@            Coverage Diff             @@
##             main     #844      +/-   ##
==========================================
+ Coverage   99.22%   99.25%   +0.03%     
==========================================
  Files          98       99       +1     
  Lines        1417     1476      +59     
  Branches      479      482       +3     
==========================================
+ Hits         1406     1465      +59     
  Misses         11       11              
Impacted Files Coverage Δ
__tests__/__util__/parserOptionsMapper.js 100.00% <ø> (ø)
src/rules/interactive-supports-focus.js 97.14% <96.66%> (+0.17%) ⬆️
__tests__/__util__/ruleOptionsMapperFactory.js 100.00% <100.00%> (ø)
src/rules/accessible-emoji.js 100.00% <100.00%> (ø)
src/rules/alt-text.js 100.00% <100.00%> (ø)
src/rules/anchor-has-content.js 100.00% <100.00%> (ø)
src/rules/anchor-is-valid.js 100.00% <100.00%> (ø)
src/rules/aria-activedescendant-has-tabindex.js 100.00% <100.00%> (ø)
src/rules/aria-role.js 100.00% <100.00%> (ø)
src/rules/aria-unsupported-elements.js 100.00% <100.00%> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1826628...64dcac6. Read the comment docs.

@jessebeach
Copy link
Collaborator

Love it, thank you for starting this.

I have a 10 week old at home and he's been a bit of a drag on time and energy for Open Source :) I'll try to find time to review, but if I'm a little slow, it's cause I'm changing diapers ;p

__tests__/src/rules/accessible-emoji-test.js Show resolved Hide resolved
src/util/getElementType.js Outdated Show resolved Hide resolved
@pcorpet pcorpet force-pushed the components branch 2 times, most recently from ef9f86b to 5bb6018 Compare March 25, 2022 10:35
src/util/getElementType.js Outdated Show resolved Hide resolved
@pcorpet pcorpet force-pushed the components branch 3 times, most recently from d46912e to 936e871 Compare March 27, 2022 19:11
README.md Outdated Show resolved Hide resolved
__tests__/src/rules/no-autofocus-test.js Outdated Show resolved Hide resolved
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!

src/util/getElementType.js Outdated Show resolved Hide resolved
@pcorpet
Copy link
Contributor Author

pcorpet commented Mar 28, 2022

I'm done updating every existing rule and helper function that was using the raw elemType function and I've added test cases for all of them I believe.

@joshuajaco
Copy link

We also need something like this, because control-has-associated-label's controlComponents option currently does not allow globs, unlike label-has-associated-control. I would greatly appreciate this being merged soon.

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!

@ljharb ljharb merged commit 64dcac6 into jsx-eslint:main Apr 8, 2022
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.

None yet

4 participants