feat(js_analyze): implement useReactAsyncServerFunction#9909
feat(js_analyze): implement useReactAsyncServerFunction#9909Netail merged 3 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 273b0c3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a nursery lint rule Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs (1)
313-333: Consider documenting the scope of export detection.The current implementation catches directly exported functions and functions exported by name, but won't flag methods within exported objects:
'use server'; export const actions = { submit() { /* not flagged */ } };If this matches upstream behaviour, a brief comment noting the intentional scope would help future maintainers. If it's unintentional, this could be a gap worth addressing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs` around lines 313 - 333, The export detection in is_exported_function only considers default exports and named exported functions via is_matching_exported_syntax / is_matching_exported_identifier, so it currently does not detect functions that are properties of exported objects (e.g., export const actions = { submit() { ... } }); update the code to either (preferred minimal change) add a short comment above is_exported_function documenting this intentional scope limitation, referencing function_binding_name, is_matching_exported_syntax, and is_matching_exported_identifier so future maintainers know object-property methods are not flagged, or (if you want to extend behavior) enhance is_exported_function to traverse exported object literals from JsExport::get_exported_items() and check object property method names against binding_name to flag such nested methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`:
- Around line 59-88: The doc-comment "Valid" examples incorrectly contain
",expect_diagnostic" in the fenced codeblock info strings; remove
",expect_diagnostic" from the three code fences that show the examples for the
JSX form action and the JS examples (the examples containing serverFunction,
serverFunction1/serverFunction2/serverFunction3) so those fences are simply
"```jsx" and "```js", ensuring the examples are treated as valid (no
diagnostics).
---
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`:
- Around line 313-333: The export detection in is_exported_function only
considers default exports and named exported functions via
is_matching_exported_syntax / is_matching_exported_identifier, so it currently
does not detect functions that are properties of exported objects (e.g., export
const actions = { submit() { ... } }); update the code to either (preferred
minimal change) add a short comment above is_exported_function documenting this
intentional scope limitation, referencing function_binding_name,
is_matching_exported_syntax, and is_matching_exported_identifier so future
maintainers know object-property methods are not flagged, or (if you want to
extend behavior) enhance is_exported_function to traverse exported object
literals from JsExport::get_exported_items() and check object property method
names against binding_name to flag such nested methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d398c7f-738f-478c-b447-77a1a0243c8c
⛔ Files ignored due to path filters (9)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (9)
.changeset/stale-ties-report.mdcrates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rscrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_react_async_server_function.rs
d7397ee to
d90a6c0
Compare
d90a6c0 to
ced4ad1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`:
- Around line 95-105: run() currently flags class constructors (e.g.,
constructor() { 'use server'; }) even though constructors cannot be async; add
an early check in run() to return None if the inspected node is a constructor
(e.g., test node.is_constructor() or inspect MethodDefinition.kind ==
"constructor") before calling has_local_use_server_directive, so constructors
are excluded from diagnostics and match the fixer behavior referenced elsewhere.
- Around line 84-85: The rule configuration in
use_react_async_server_function.rs currently sets fix_kind: FixKind::Safe even
though the autofix inserts an async keyword and changes a function's return
contract; change the rule's fix_kind to FixKind::Unsafe so this
behavior-changing fix isn't applied automatically—locate the rule definition
where sources: &[RuleSource::EslintReact("async-server-action")... ] and replace
FixKind::Safe with FixKind::Unsafe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ebb274c3-ba71-47e1-9b0d-216355b533eb
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs
e28195b to
f19a759
Compare
f19a759 to
1a1b6f7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs (1)
95-105:⚠️ Potential issue | 🟡 MinorConstructors should be excluded in
run()to avoid unfixable diagnostics.The
action()function correctly bails on constructors, butrun()will emit a diagnostic if a constructor has a'use server'directive. Since JavaScript constructors cannot beasync, users get an unfixable warning.Add the constructor check early in
run():fn run(ctx: &RuleContext<Self>) -> Self::Signals { let node = ctx.query(); + if matches!(node, AnyFunctionLike::JsConstructorClassMember(_)) { + return None; + } + // Generator functions are ignored, matching the upstream behavior. if node.is_generator() || node.is_async() { return None; }Also consider adding a test case confirming constructors with
'use server'are correctly ignored (none currently exist in the test files).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs` around lines 95 - 105, The run() function is emitting diagnostics for constructor methods with a 'use server' directive; add an early check in run() to return None when node.is_constructor() (mirroring the action() guard) so constructors are excluded; update the run() function (referencing run, node.is_constructor(), has_local_use_server_directive, and ServerFunctionKind) and add a unit test ensuring constructors with 'use server' are ignored to prevent unfixable warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`:
- Around line 95-105: The run() function is emitting diagnostics for constructor
methods with a 'use server' directive; add an early check in run() to return
None when node.is_constructor() (mirroring the action() guard) so constructors
are excluded; update the run() function (referencing run, node.is_constructor(),
has_local_use_server_directive, and ServerFunctionKind) and add a unit test
ensuring constructors with 'use server' are ignored to prevent unfixable
warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 518b1e43-07a3-4083-ba21-5b10c1a22f82
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/invalid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rscrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`:
- Around line 301-312: The current export-matching uses only the identifier text
(via function_binding_name and is_matching_exported_identifier) which falsely
matches nested bindings; update the check to resolve the exported identifier to
its actual declaration node (walk to the declaration/parent chain) and compare
that node to the function's top-level binding instead of comparing text
only—follow the safer pattern used in use_qwik_loader_location.rs (resolve
exported identifier to its declarator/parent and compare node identity) and
replace the is_matching_exported_identifier/name-only path in the root.syntax()
JsExport traversal with this resolved-declaration comparison for both the block
around function_binding_name/is_matching_exported_identifier and the similar
code at 333-347.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04363590-01f1-40b8-8e23-655cc7acd5e4
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/file/invalid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rscrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/valid.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useReactAsyncServerFunction/block/invalid.jsx
| let binding_name = function_binding_name(function); | ||
|
|
||
| root.syntax() | ||
| .descendants() | ||
| .filter_map(JsExport::cast) | ||
| .flat_map(|export| export.get_exported_items()) | ||
| .any(|item| { | ||
| item.exported.as_ref().is_some_and(|exported| { | ||
| is_matching_exported_syntax(function, exported) | ||
| || is_matching_exported_identifier(binding_name.as_ref(), exported) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Name-only export matching can hit the wrong function.
Line 310 treats any function whose binding text matches an exported identifier as exported, even when that binding lives in a nested scope. In a file-level 'use server' module, function wrapper() { const action = () => {}; } export { action }; would end up diagnosing the inner action, which is a bit too eager even for a nursery rule. Please resolve the export against the actual top-level declaration (or parent chain), not just the identifier text; crates/biome_js_analyze/src/lint/nursery/use_qwik_loader_location.rs:1-80 already uses that safer pattern for declarators.
Also applies to: 333-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/use_react_async_server_function.rs`
around lines 301 - 312, The current export-matching uses only the identifier
text (via function_binding_name and is_matching_exported_identifier) which
falsely matches nested bindings; update the check to resolve the exported
identifier to its actual declaration node (walk to the declaration/parent chain)
and compare that node to the function's top-level binding instead of comparing
text only—follow the safer pattern used in use_qwik_loader_location.rs (resolve
exported identifier to its declarator/parent and compare node identity) and
replace the is_matching_exported_identifier/name-only path in the root.syntax()
JsExport traversal with this resolved-declaration comparison for both the block
around function_binding_name/is_matching_exported_identifier and the similar
code at 333-347.
Summary
Port
rsc-function-definition&async-server-action, which require React server functions to be asyncGenerated with the help of co-pilot
Test Plan
Docs