Add oxlint plugin to @fedify/lint#760
Open
nyanrus wants to merge 8 commits intofedify-dev:mainfrom
Open
Conversation
Expose the existing rule set through oxlint's JS plugin API at the new `@fedify/lint/oxlint` subpath export. The same rules used by the ESLint plugin are reused verbatim — oxlint claims an ESLint-compatible API, so no rule logic is duplicated. Distributed via npm only since oxlint's JS plugin loader runs only under Node.js. Includes an integration test that spawns the oxlint binary against a tmpdir fixture, plus an example project under examples/lint/oxlint/ that demonstrates the configuration with both a violating and a corrected fixture. Assisted-by: Claude Code:claude-opus-4-7
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an Oxlint subpath export and plugin implementation for ChangesOxlint Support Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/lint/oxlint/README.md`:
- Around line 6-8: Add a short stability caveat to the README.md's intro
paragraph explaining that Oxlint's JavaScript plugin support is currently
upstream alpha; update the opening lines where the example is described (the
paragraph referencing `@fedify/lint` and [oxlint]) to append a single-sentence
note like "Note: Oxlint's JS plugin support is upstream alpha and may be
unstable" so users are aware of adoption risk.
In `@packages/lint/src/tests/oxlint.test.ts`:
- Line 17: The test import uses the Node-only harness; replace the
runtime-specific import by importing the test harness from the repo-standard
package. Update the import statement that currently reads importing "test"
(symbol: test) from "node:test" to import "test" from "@fedify/fixture" so this
file (oxlint.test.ts) uses the repository-standard, runtime-agnostic test
harness.
- Around line 75-120: Wrap the temporary-dir usage in the test in a try/finally:
create dir with mkdtempSync as now, run the test logic (writes, spawnSync,
assertions) inside try, and in finally call rmSync(dir, { recursive: true,
force: true }) to always remove the fixture. If rmSync is not imported, add it
from 'fs' (or call fs.rmSync) so the cleanup compiles; reference the local
variable dir in the finally block and keep the existing assertions and error
handling unchanged.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b56fc3b9-8fb2-468b-b50d-f1ec0a5eccb9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
docs/manual/lint.mdexamples/lint/oxlint/.oxlintrc.jsonexamples/lint/oxlint/README.mdexamples/lint/oxlint/federation.fixed.tsexamples/lint/oxlint/federation.tsexamples/lint/oxlint/package.jsonpackages/lint/README.mdpackages/lint/package.jsonpackages/lint/src/oxlint.tspackages/lint/src/tests/oxlint.test.tspackages/lint/tsdown.config.tspnpm-workspace.yaml
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for oxlint, a fast Rust-based linter, to the @fedify/lint package. It adds a new subpath export, @fedify/lint/oxlint, which exposes the existing Fedify lint rules in a format compatible with oxlint's JS plugin API. The changes include comprehensive documentation updates, a new example project demonstrating oxlint integration, and an integration test suite. Feedback from the reviewer identifies a documentation issue where a JSON code block containing comments should be labeled as jsonc to ensure syntactic correctness.
dahlia
requested changes
May 10, 2026
Member
dahlia
left a comment
dahlia
left a comment
There was a problem hiding this comment.
Left several nitpicking comments. I guess @2chanhaeng, the original author of @fedify/lint, could give you more helpful feedback.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
The oxlint section of the lint package README and the linting manual both warned that the JS plugin loader "runs only under Node.js" and therefore the `@fedify/lint/oxlint` subpath was distributed via npm only. That claim is not supported by the oxlint docs — it conflated a note about TypeScript config files (which do require the Node-based oxlint package) with the JS plugin mechanism, which has no such restriction documented. Drop the misleading sentence, expose the same subpath through JSR via deno.json, and migrate the integration test to `@fedify/fixture`'s runtime-agnostic `test()` so it runs under both `pnpm test` (node:test) and `deno task test` (Deno.test) with the same source. The findOxlint helper now also checks the package-local `node_modules/.bin` so it can locate the binary under pnpm's hoisting layout in addition to the workspace-root and PATH lookups. Refs: fedify-dev#702 Assisted-by: Claude Code:claude-opus-4-7
Match the casing the rest of the docs already use for product names like Deno Lint and ESLint. Lowercase usages are kept where they refer to the binary, the config filename, or the package subpath (`@fedify/lint/oxlint`). The hongdown proper_nouns list gets an "Oxlint" entry so the formatter preserves the casing in headings on subsequent runs. Refs: fedify-dev#702 Assisted-by: Claude Code:claude-opus-4-7
The example README's intro previously didn't mention the upstream alpha status, so users landing directly on the example missed the caveat that the package and manual docs carry. Append a single sentence to the intro paragraph so adoption risk is visible at first read. Refs: fedify-dev#702 Assisted-by: Claude Code:claude-opus-4-7
The example uses a leading `// .oxlintrc.json` comment to label the filename, which is invalid JSON; the surrounding Deno Lint and ESLint examples avoid the issue by tagging their blocks `typescript`. Tagging this one `jsonc` matches the actual content (JSON with comments). Refs: fedify-dev#702 Assisted-by: Claude Code:claude-opus-4-7
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/lint/src/tests/oxlint.test.ts (1)
72-121:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClean up the temporary fixture directory after the test.
The test creates a temp directory at line 76 but never removes it, which will accumulate temp directories in local dev and CI environments over time.
🧹 Suggested cleanup patch
-import { existsSync, mkdtempSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";() => { const dir = mkdtempSync(join(tmpdir(), "fedify-lint-oxlint-")); - writeFileSync( - join(dir, ".oxlintrc.json"), - JSON.stringify({ - jsPlugins: [pluginPath], - rules: { - "@fedify/lint/actor-id-required": "error", - }, - }), - ); - writeFileSync(join(dir, "federation.ts"), BAD_CODE); - - const result = spawnSync( - oxlintBin!, - ["--format=json", "."], - { cwd: dir, encoding: "utf8" }, - ); - - ok( - result.status !== 0, - `Expected non-zero exit, got ${result.status}. stderr: ${result.stderr}`, - ); - - let payload: OxlintJsonReport; - try { - payload = JSON.parse(result.stdout) as OxlintJsonReport; - } catch (err) { - throw new Error( - `Failed to parse oxlint JSON output: ${(err as Error).message}\n` + - `stdout: ${result.stdout}\nstderr: ${result.stderr}`, - ); - } - - const codes = (payload.diagnostics ?? []).map((d) => d.code ?? ""); - const matched = codes.some((code) => - code === "@fedify/lint(actor-id-required)" || - code.includes("actor-id-required") - ); - ok( - matched, - `Expected `@fedify/lint`(actor-id-required) diagnostic, got: ${ - codes.join(", ") || "(none)" - }`, - ); + try { + writeFileSync( + join(dir, ".oxlintrc.json"), + JSON.stringify({ + jsPlugins: [pluginPath], + rules: { + "@fedify/lint/actor-id-required": "error", + }, + }), + ); + writeFileSync(join(dir, "federation.ts"), BAD_CODE); + + const result = spawnSync( + oxlintBin!, + ["--format=json", "."], + { cwd: dir, encoding: "utf8" }, + ); + + ok( + result.status !== 0, + `Expected non-zero exit, got ${result.status}. stderr: ${result.stderr}`, + ); + + let payload: OxlintJsonReport; + try { + payload = JSON.parse(result.stdout) as OxlintJsonReport; + } catch (err) { + throw new Error( + `Failed to parse oxlint JSON output: ${(err as Error).message}\n` + + `stdout: ${result.stdout}\nstderr: ${result.stderr}`, + ); + } + + const codes = (payload.diagnostics ?? []).map((d) => d.code ?? ""); + const matched = codes.some((code) => + code === "@fedify/lint(actor-id-required)" || + code.includes("actor-id-required") + ); + ok( + matched, + `Expected `@fedify/lint`(actor-id-required) diagnostic, got: ${ + codes.join(", ") || "(none)" + }`, + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lint/src/tests/oxlint.test.ts` around lines 72 - 121, The test creates a temporary directory in the local variable "dir" via mkdtempSync but never removes it; wrap the test body in a try/finally (or add a teardown) and remove the temp directory in the finally block using fs.rmSync(dir, { recursive: true, force: true }) (or equivalent cleanup) to ensure the directory created by mkdtempSync is always deleted after the test (keep references to the existing symbols: mkdtempSync, dir, writeFileSync, spawnSync, and the test named "oxlint plugin: actor-id-required fires on missing id").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/lint/src/tests/oxlint.test.ts`:
- Around line 72-121: The test creates a temporary directory in the local
variable "dir" via mkdtempSync but never removes it; wrap the test body in a
try/finally (or add a teardown) and remove the temp directory in the finally
block using fs.rmSync(dir, { recursive: true, force: true }) (or equivalent
cleanup) to ensure the directory created by mkdtempSync is always deleted after
the test (keep references to the existing symbols: mkdtempSync, dir,
writeFileSync, spawnSync, and the test named "oxlint plugin: actor-id-required
fires on missing id").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e7360ef-6edd-4b6d-8e68-5eaba546c321
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.hongdown.tomldocs/manual/lint.mdexamples/lint/oxlint/README.mdpackages/lint/README.mdpackages/lint/deno.jsonpackages/lint/package.jsonpackages/lint/src/tests/oxlint.test.ts
Two review points on packages/lint/src/tests/oxlint.test.ts:
- Make the runtime story explicit. The test imports `node:*` modules
and spawns the oxlint binary, which is unusual for this package
compared with the in-process Deno.lint / ESLint Linter tests. The
header comment now spells out that Deno's node compat layer is what
keeps the same source running under both `pnpm test` and
`deno task test`, that subprocess-spawning is intentional (oxlint
is a Rust binary), and that the binary plus the built loader are
ignore-skip preconditions.
- Clean up the temp fixture directory. Wrap the test body in
`try/finally` so `rmSync(dir, { recursive: true, force: true })`
runs whether the assertions pass or throw. Avoids local/CI temp
buildup.
Refs: fedify-dev#760
Assisted-by: Claude Code:claude-opus-4-7
Co-authored-by: Hong Minhee (洪 民憙) <hong@minhee.org>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/manual/lint.md`:
- Line 318: Remove the unused reference-style link definition "[Rules]: `#rules`"
from docs/manual/lint.md; locate the literal "[Rules]: `#rules`" (the reference
link definition) and delete that line since the page already uses an inline
anchor "( *Rules* section](`#rules`)" and the reference is unused.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dedc1a61-f7cd-4198-ad44-257bc24cc719
📒 Files selected for processing (1)
docs/manual/lint.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #702
Expose the existing rule set through oxlint's JS plugin API at the new
@fedify/lint/oxlintsubpath export. The same rules used by the ESLint plugin are reused verbatim — oxlint claims an ESLint-compatible API, so no rule logic is duplicated.Includes an integration test that spawns the oxlint binary against a tmpdir fixture, plus an example project under examples/lint/oxlint/ that demonstrates the configuration with both a violating and a corrected fixture.
edit:
Deno support added!