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-static-element-interactions: allow role assignments using a ternary with literals on both sides #865

Merged
merged 1 commit into from Jul 5, 2022

Conversation

V2dha
Copy link
Contributor

@V2dha V2dha commented Jul 1, 2022

Fixes 766

…a ternary with literals on both sides

Fixes jsx-eslint#766.

Co-authored-by: Vividha <rvividha@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
@V2dha V2dha changed the title Specific case for role assignment using ternary with literals on both side Role assignment using ternary with literals on both side Jul 1, 2022
@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #865 (5285b91) into main (2362832) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 5285b91 differs from pull request most recent head 7524e0c. Consider uploading reports for the commit 7524e0c to get more accurate results

@@           Coverage Diff           @@
##             main     #865   +/-   ##
=======================================
  Coverage   99.25%   99.25%           
=======================================
  Files          98       98           
  Lines        1474     1481    +7     
  Branches      482      487    +5     
=======================================
+ Hits         1463     1470    +7     
  Misses         11       11           
Impacted Files Coverage Δ
src/rules/no-static-element-interactions.js 100.00% <100.00%> (ø)

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 2362832...7524e0c. Read the comment docs.

@ljharb
Copy link
Member

ljharb commented Jul 2, 2022

I rebased this and added a test case, which fails both without your fix and with it.

@ljharb ljharb force-pushed the ternary-with-literals-role branch from 6ab6d89 to 220766d Compare July 2, 2022 05:21
@V2dha
Copy link
Contributor Author

V2dha commented Jul 3, 2022

@ljharb Can you review the code that I added once? I feel like it is not able to access the prop value of the role attribute? Does the getPropValue function only returns the value and not the node of the prop?

@ljharb
Copy link
Member

ljharb commented Jul 5, 2022

It kind of seems like getProp(attributes, 'role') returns a JSXAttribute node, and getPropValue is returning "button", so you can't figure out that it's a conditional expression?

@ljharb
Copy link
Member

ljharb commented Jul 5, 2022

@V2dha i pushed up a commit that i think will fix it; what do you think?

I also filed #867 since the allowExpressionValues option is undocumented.

@V2dha
Copy link
Contributor Author

V2dha commented Jul 5, 2022

Thanks for the fix! Tests are passing locally now. I tried using getProp function earlier as well but it was throwing error while getting its type maybe because it was returning a JSXAttribute node.

@V2dha V2dha marked this pull request as ready for review July 5, 2022 19:46
@ljharb ljharb force-pushed the ternary-with-literals-role branch from 5285b91 to 7524e0c Compare July 5, 2022 20:46
@ljharb ljharb changed the title Role assignment using ternary with literals on both side [Fix] no-static-element-interactions: allow role assignments using a ternary with literals on both sides Jul 5, 2022
@ljharb ljharb merged commit 7524e0c into jsx-eslint:main Jul 5, 2022
@V2dha V2dha deleted the ternary-with-literals-role branch July 6, 2022 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-static-element-interactions cannot be resolved with dynamically-defined roles
2 participants