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(lint/useJsxKeyInIterable): don't trigger useJsxKeyInIterable when iterating on non-jsx things #2667

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented May 1, 2024

Summary

This aims to resolve some of the bugs in #2590. It's intentionally small in scope since this is my first PR here, so hopefully it will be easier to review.

It fixes cases where expressions like this would erroneously trigger the useJsxKeyInIterable lint:

<>{data.reduce((total, next) => total + next, 0)}</>

related to: #2590

Test Plan

I added tests.

cargo test -p biome_js_analyze use_jsx_key_in_iterable

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels May 1, 2024
@dyc3 dyc3 marked this pull request as ready for review May 1, 2024 14:16
@dyc3 dyc3 changed the title fix(lint): don't trigger useJsxKeyInIterable when iterating on non-jsx things fix(lint/useJsxKeyInIterable): don't trigger useJsxKeyInIterable when iterating on non-jsx things May 1, 2024
@Conaclos Conaclos requested a review from a team May 1, 2024 15:24
@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from 0f86ef9 to 25e483a Compare May 1, 2024 15:39
Copy link

codspeed-hq bot commented May 1, 2024

CodSpeed Performance Report

Merging #2667 will not alter performance

Comparing dyc3:05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things (644a6ed) with main (43525a3)

Summary

✅ 86 untouched benchmarks

@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from 25e483a to d524940 Compare May 1, 2024 17:16
@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from d524940 to 5f72d5f Compare May 2, 2024 16:40
@dyc3 dyc3 requested a review from vohoanglong0107 May 2, 2024 16:41
@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from 5f72d5f to 65a06f2 Compare May 2, 2024 16:42
@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from 65a06f2 to bbecf2b Compare May 3, 2024 11:53
@dyc3 dyc3 force-pushed the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch from bbecf2b to 644a6ed Compare May 3, 2024 12:54
@dyc3
Copy link
Contributor Author

dyc3 commented May 3, 2024

I've moved the conditional expression check to another branch that I have locally, and I'll post the PR for that once this is merged.

@dyc3 dyc3 requested a review from vohoanglong0107 May 3, 2024 13:06
@ematipico ematipico merged commit 1b63093 into biomejs:main May 3, 2024
13 checks passed
@ematipico
Copy link
Member

Feel free to leave possible comments @vohoanglong0107, in case there's something missing

@dyc3 dyc3 deleted the 05-01-fix_lint_don_t_trigger_usejsxkeyiniterable_when_iterating_on_non-jsx_things branch May 3, 2024 19:11
@Conaclos Conaclos added the A-Changelog Area: changelog label May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants