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

feat(linter): add jsx support for only-used-in-recursion #7120

Merged
merged 19 commits into from
Nov 9, 2024

Conversation

no-yan
Copy link
Contributor

@no-yan no-yan commented Nov 4, 2024

close #5530

This PR adds support for handling recursive JSX components, allowing detect functions like the one below:

function ListItem({ depth }) {
    return <ListItem depth={depth} />
}

Some logic is duplicated with the non-JSX case. But the refactor will be complicated and affects over-all, so we can make it in another PR.

cc: @DonIsaac @camc314

Copy link

graphite-app bot commented Nov 4, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the A-linter Area - Linter label Nov 4, 2024
@no-yan no-yan changed the title feat: add jsx support for only-used-in-recursion feat(linter): add jsx support for only-used-in-recursion Nov 4, 2024
@github-actions github-actions bot added the C-enhancement Category - New feature or request label Nov 4, 2024
Copy link

codspeed-hq bot commented Nov 4, 2024

CodSpeed Performance Report

Merging #7120 will not alter performance

Comparing no-yan:feat/support-jsx-in-only-recursion (be72243) with main (e536d47)

Summary

✅ 30 untouched benchmarks

@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from ff8289f to 9bbc8fd Compare November 4, 2024 15:30
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

this is awesome, thank you! i will review properly later

@camc314
Copy link
Contributor

camc314 commented Nov 4, 2024

This looks good to me i think we can re-use a few exising fns that are already impl-ed though

@no-yan
Copy link
Contributor Author

no-yan commented Nov 5, 2024

@camc314 Thank you for the review. I’ve made the suggested changes.

I'll rebase fix-up commit after this PR gets approval.

@no-yan no-yan requested a review from camc314 November 5, 2024 14:03
@no-yan
Copy link
Contributor Author

no-yan commented Nov 5, 2024

I went ahead and sent an explicit review request, but no rush—take your time! :)

@camc314 camc314 requested review from Boshen and DonIsaac November 5, 2024 14:45
@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from 14aa74c to 90791de Compare November 5, 2024 15:10
@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from 90791de to 23a0533 Compare November 5, 2024 15:31
@no-yan

This comment was marked as resolved.

@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from 23a0533 to 14aa74c Compare November 5, 2024 15:39
@camc314
Copy link
Contributor

camc314 commented Nov 5, 2024

@no-yan see #7126

@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from 14aa74c to f791c81 Compare November 5, 2024 16:14
@github-actions github-actions bot added A-semantic Area - Semantic A-ast Area - AST A-transformer Area - Transformer / Transpiler A-ast-tools Area - AST tools labels Nov 5, 2024
@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from f791c81 to bb2dabf Compare November 5, 2024 16:22
@no-yan
Copy link
Contributor Author

no-yan commented Nov 5, 2024

The build is now successful with commit: bb2dabf
@camc314 Thank you for always kindly helping me.

Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

thank you for contributing!

@camc314 camc314 requested a review from camchenry November 5, 2024 22:12
@camc314 camc314 requested a review from camchenry November 7, 2024 16:59
Copy link
Contributor

@camchenry camchenry left a comment

Choose a reason for hiding this comment

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

thanks for your contribution!

crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs Outdated Show resolved Hide resolved
crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs Outdated Show resolved Hide resolved
@no-yan no-yan force-pushed the feat/support-jsx-in-only-recursion branch from aa4878c to c42d5ed Compare November 9, 2024 07:48
@Boshen Boshen merged commit 1fcd709 into oxc-project:main Nov 9, 2024
27 checks passed
@oxc-bot oxc-bot mentioned this pull request Nov 9, 2024
Boshen added a commit that referenced this pull request Nov 9, 2024
## [0.11.1] - 2024-11-09

### Features

- 4dd9b60 editor/vscode: Replace existing output channel and trace
output channel with a single LogOutputChannel (#7196) (Nicholas Rayburn)
- 1fcd709 linter: Add jsx support for only-used-in-recursion (#7120)
(no-yan)
- 4d577cf linter: Add `import/first` rule (#7180) (Dmitry Zakharov)
- 9b8973f linter: Add `import/unambiguous` rule (#7187) (Dmitry
Zakharov)
- 5ab1ff6 linter: Implement @typescript-eslint/no-unsafe-function-type
(#6989) (Orenbek)

### Bug Fixes

- eea8879 editor/vscode: Update language client id to fix the resolution
of the oxc.trace.server setting (#7181) (Nicholas Rayburn)
- b73cfd9 linter: Fix `is_method_call` with parentheses and chain
expression (#7095) (tbashiyy)

### Refactor

- 8c0a362 linter: Use `ctx.source_range(comment.content_span())` API
(#7155) (Boshen)
- c5485ae semantic: Add `ancestor_kinds` iterator function (#7217)
(camchenry)
- abf1602 semantic: Rename `iter_parents` to `ancestors` (#7216)
(camchenry)
- 42171eb semantic: Rename `ancestors` to `ancestor_ids` (#7215)
(camchenry)

Co-authored-by: Boshen <[email protected]>
@camc314
Copy link
Contributor

camc314 commented Nov 9, 2024

@no-yan great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-ast-tools Area - AST tools A-linter Area - Linter A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enhancement(linter) expand only-used-in-recursion to support jsx
4 participants