Skip to content

fix(linter/react/jsx-handler-names): do not detect the function name within the inline-function's body block#13456

Merged
camc314 merged 3 commits intooxc-project:mainfrom
takuji:fix-jsx-handler-names-detect-handler-within-function-body-block
Sep 15, 2025
Merged

fix(linter/react/jsx-handler-names): do not detect the function name within the inline-function's body block#13456
camc314 merged 3 commits intooxc-project:mainfrom
takuji:fix-jsx-handler-names-detect-handler-within-function-body-block

Conversation

@takuji
Copy link
Contributor

@takuji takuji commented Aug 30, 2025

This PR fixes a behavior of react/jsx-handler-names rule incompatible with that of the eslint-version.

Fixed difference with the eslint version

With the configuration below:

{ "checkInlineFunction": true, "checkLocalVariables": true }

the eslint version of react/jsx-handler-names rule treats the following code as invalid.

<TestComponent onChange={() => { handleChange() }} />

But the current oxc version finds handleChange to be an event handler name and considers this code to be correct.

Along with this change, the diagnostic messages are also modified because the event handler name is not always available, and replaced "invalid" with "bad" to be in accordance with the eslint-version's message IDbadHandlerName and badPropKey.

Copilot AI review requested due to automatic review settings August 30, 2025 21:27
@takuji takuji requested a review from camc314 as a code owner August 30, 2025 21:27
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 30, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Aug 30, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the behavior of the react/jsx-handler-names rule to align with the ESLint version by preventing the detection of function names within inline function body blocks. When configured with checkInlineFunction: true and checkLocalVariables: true, the rule should only analyze expression arrow functions, not those with block statements.

  • Updates diagnostic messages from "Invalid" to "Bad" to match ESLint's message IDs
  • Adds early return for arrow functions with block body statements to prevent false positives
  • Simplifies function signatures by removing unused parameters

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
jsx_handler_names.rs Adds block body check, updates diagnostic functions, and adds test cases
react_jsx_handler_names.snap Updates expected diagnostic messages from "Invalid" to "Bad"

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 30, 2025

CodSpeed Instrumentation Performance Report

Merging #13456 will not alter performance

Comparing takuji:fix-jsx-handler-names-detect-handler-within-function-body-block (c0e3375) with main (48fde64)

Summary

✅ 37 untouched

@takuji takuji force-pushed the fix-jsx-handler-names-detect-handler-within-function-body-block branch from 845b8bc to d200e65 Compare August 30, 2025 22:46
…called within a function body block

Also modify the diagnostic message because handler names are not always available.
@takuji takuji force-pushed the fix-jsx-handler-names-detect-handler-within-function-body-block branch from e426ba4 to 271a4eb Compare September 14, 2025 20:17
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!

@camc314 camc314 merged commit 56da114 into oxc-project:main Sep 15, 2025
26 checks passed
This was referenced Sep 16, 2025
camc314 added a commit that referenced this pull request Sep 16, 2025
## [1.16.0] - 2025-09-16

### 🚀 Features

- 97c8d06 linter: Add `preserve-caught-error` rule (#13748) (孔辉)
- 8c19b18 linter/exhaustive-deps: Implement fixer for dep in global
scope (#13783) (camc314)
- 06bce8f linter/exhaustive-deps: Implement fixer for missing dep
(#13782) (camc314)
- a8675f4 linter: Add eslint/class-methods-use-this rule (#12977) (Peter
Cardenas)
- db33196 parser: Adds typescript rule for empty argument list (#13730)
(Karan Kiri)
- 2751193 linter: Add `eslint/no-useless-computed-key` rule (#13428)
(yefan)
- 9a205d1 regex-parser: Parse simple `TemplateLiterals` (#13265) (Sysix)

### 🐛 Bug Fixes

- a2c91cd linter: Drop `rules` to allow mutable access to `ctx_host` in
`run_external_rules` (#13832) (camc314)
- 3af1e5d linter/no-unsafe-declaration-merging: Always mark first span
as primary (#13830) (camc314)
- 1c43c7c linter: Keep message when merging composite fixes (#13827)
(camc314)
- 26af302 linter/exhaustive-deps: Check stable value is on lhs of
assignment expr (#13815) (camc314)
- 4bc12d0 linter/exhaustive-deps: Remove impossible comparison with
parent kind (#13814) (camc314)
- 12baf5e linter/exhaustive-deps: Respect primary span when identifying
disable directive location (#13781) (camc314)
- fa7400a linter/no-undef: False positive with `arguments` in functions
(#13763) (camc314)
- 50e6e3c editor: Restrict servers paths for `oxc.path.server` (#13740)
(Sysix)
- b45077d editor: Strip leading slash for bin path on windows (#13738)
(Sysix)
- 8fa6227 editor: Don't allow `oxc.path.server` for untrusted workspaces
(#13734) (Sysix)
- 56da114 linter/react/jsx-handler-names: Do not detect the function
name within the inline-function's body block (#13456) (Takuji Shimokawa)
- b2bc5b4 linter/react-perf/jsx-no-new-object-as-prop: Skip as/satisfies
exprs (#13718) (camc314)
- ab51394 raw_transfer: Disable layout assertions on some 32-bit
platforms (#13716) (overlookmotel)
- 09428f6 linter/plugins: Remove outdated comment (#13691)
(overlookmotel)
- a294721 linter/plugins: Exit early if JS plugins enabled on
unsupported platforms (#13689) (overlookmotel)
- 68a2280 linter/plugins: More graceful exit for
`--experimental-js-plugins` CLI option (#13688) (overlookmotel)

### 🚜 Refactor

- 395d40d linter: Derive inmpls for `PartialEq`, `Eq` over manual ones
(#13828) (camc314)
- 8e4cd8f linter/func-names: Use `run_once` over looping over all nodes
(#13798) (camc314)
- 7f4e2fe eslint/func-names: Clean up implementation and improve
documentation (#13601) (Antoine Zanardi)
- 137896a language_server: Split options for linting and formatting
(#13627) (Sysix)
- 7346099 linter: Move `oxlint` application code into separate module
(#13745) (overlookmotel)
- 6dd4107 linter: Remove `#[cfg(test)]` attributes from `tester` module
(#13714) (overlookmotel)
- c40c6ef linter/plugins: Directory for JS plugins-related code (#13701)
(overlookmotel)
- a0022c1 linter/plugins: Improve error messages for JS plugins (#13699)
(overlookmotel)
- 1fd993f napi/oxlint: Rename `napi/oxlint2` to `napi/oxlint` (#13682)
(overlookmotel)

### ⚡ Performance

- 90c8286 linter: Detect node types from `let..else` statements (#13690)
(camchenry)
- 08c05df semantic: Make CFG construction a compile-time feature
(#13678) (Boshen)

### 🎨 Styling

- 99a7638 linter: Add comments + re-organise imports (#13715)
(overlookmotel)

### 🧪 Testing

- 18a1145 linter: Add debug assertions for skipping rules (#13724)
(camc314)
- cb080de linter/no-unused-vars: Add test for non ASCII chars in JSX
components (#13820) (camc314)
- b6eba27 linter/no-undef: Add more test cases for `arguments` (#13764)
(camc314)
- fb2d087 linter: Set CWD for tests (#13722) (overlookmotel)

Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants