feat!: export JS config with defineConfig() for oxlint >= 1.51.0#2
feat!: export JS config with defineConfig() for oxlint >= 1.51.0#2
Conversation
BREAKING CHANGE: The default export is now a JS config object wrapped
with defineConfig() from oxlint. This is required by oxlint >= 1.51.0
which no longer supports string paths in extends.
Usage in vite.config.ts:
import oxlintConfig from '@eggjs/oxlint-config';
lint: { extends: [oxlintConfig] }
The .oxlintrc.json is still available for direct CLI use.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a breaking change to the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughA new oxlint configuration module (index.mjs) is introduced and exported via package.json, with the package version bumped to 2.0.0. README documentation is streamlined by removing badges and adding concrete configuration code examples for modern JavaScript/TypeScript setups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a breaking change to export the oxlint configuration as a JavaScript object using defineConfig(), to support oxlint >= 1.51.0. The changes are well-structured, including a new index.mjs entry point, corresponding updates to package.json, and revised documentation in README.md. My review includes a couple of minor suggestions for the new index.mjs configuration file to improve its clarity and maintainability.
| 'no-empty-function': 'error', | ||
| 'max-depth': ['error', 6], | ||
| 'max-lines-per-function': 'allow', | ||
| 'no-lonely-if': 'error', |
There was a problem hiding this comment.
The no-lonely-if rule is also provided by the unicorn plugin as unicorn/no-lonely-if (which you've enabled on line 115). The unicorn version is an enhanced version with autofix capabilities. To avoid redundancy, it's recommended to remove this core ESLint rule and rely solely on the unicorn version.
| 'unicorn/prefer-number-properties': 'error', | ||
| 'unicorn/prefer-array-some': 'error', | ||
| 'unicorn/prefer-string-slice': 'error', | ||
| // "unicorn/no-null": "error", |
There was a problem hiding this comment.
This commented-out rule appears to be a leftover. The unicorn/no-null rule is already configured as 'allow' on line 99. To improve code clarity and avoid confusion, this redundant line should be removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
index.mjs (1)
3-150: Avoid maintaining the same config in two places.
index.mjsnow duplicates the full payload from.oxlintrc.json. That makes future rule changes easy to miss in one entry point and silently diverge behavior between the JS export and the legacy JSON export.Suggested direction
Author one canonical config and generate the legacy artifact from it during publish. For example, keep
index.mjsas the source of truth and add a small release script that writes.oxlintrc.jsonfrom the same object before publish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.mjs` around lines 3 - 150, index.mjs duplicates the full rule payload from .oxlintrc.json, causing drift; instead make a single source of truth and generate the legacy JSON during release. Keep the exported config object used by defineConfig (the default export in index.mjs) as the canonical config, remove the duplicated .oxlintrc.json from manual maintenance, and add a small release/build script (e.g., a publish script or prepack step) that imports/reads the same exported config and writes it to .oxlintrc.json before publishing; ensure the script serializes the same object shape (rules, plugins, categories, ignorePatterns) so both consumers get identical configs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.mjs`:
- Line 26: Several inline comment headers like "// eslint" are failing the
capitalized-comments rule; update each section header comment (e.g., the comment
text "// eslint" and the other comment headers at the noted locations) to start
with an uppercase letter (for example change "// eslint" to "// ESLint") so they
are capitalized; locate these comment lines in index.mjs and edit their initial
letter to be uppercase for each occurrence.
In `@package.json`:
- Around line 36-42: The package currently marks the peerDependency "oxlint" as
optional while the package's runtime (index.mjs) does a direct import of oxlint,
which will cause MODULE_NOT_FOUND for JS consumers; update package.json by
removing the "optional": true entry under "peerDependenciesMeta" for "oxlint"
(or otherwise ensure "oxlint" is a required peerDependency) so that installing
this package will require oxlint to be installed, and keep README notes about
JSON-only usage if you still want to support consumers that avoid the JS API.
---
Nitpick comments:
In `@index.mjs`:
- Around line 3-150: index.mjs duplicates the full rule payload from
.oxlintrc.json, causing drift; instead make a single source of truth and
generate the legacy JSON during release. Keep the exported config object used by
defineConfig (the default export in index.mjs) as the canonical config, remove
the duplicated .oxlintrc.json from manual maintenance, and add a small
release/build script (e.g., a publish script or prepack step) that imports/reads
the same exported config and writes it to .oxlintrc.json before publishing;
ensure the script serializes the same object shape (rules, plugins, categories,
ignorePatterns) so both consumers get identical configs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9267ed9-c171-4f55-aa59-abc6b8c51ca7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
README.mdindex.mjspackage.json
| 'oxc', | ||
| ], | ||
| rules: { | ||
| // eslint |
There was a problem hiding this comment.
Capitalize these section headers to unblock CI.
These comments currently trip capitalized-comments, so both workflows stay red until they are uppercased.
Proposed fix
- // eslint
+ // ESLint
...
- // maybe warn
+ // Maybe warn
...
- // import
+ // Import
...
- // promise
+ // Promise
...
- // unicorn
+ // Unicorn
...
- // oxc
+ // Oxc
...
- // typescript
+ // TypeScript
...
- // jsdoc
+ // JSDocAlso applies to: 54-54, 73-73, 88-88, 97-97, 123-123, 130-130, 146-146
🧰 Tools
🪛 GitHub Actions: Node.js CI
[error] 26-26: eslint(capitalized-comments): Comments should not begin with a lowercase letter. Help: Change the first letter of the comment to uppercase.
🪛 GitHub Actions: Publish Any Commit
[error] 26-26: ESLint: Comments should not begin with a lowercase letter (capitalized-comments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.mjs` at line 26, Several inline comment headers like "// eslint" are
failing the capitalized-comments rule; update each section header comment (e.g.,
the comment text "// eslint" and the other comment headers at the noted
locations) to start with an uppercase letter (for example change "// eslint" to
"// ESLint") so they are capitalized; locate these comment lines in index.mjs
and edit their initial letter to be uppercase for each occurrence.
| "peerDependencies": { | ||
| "oxlint": ">=1.51.0" | ||
| }, | ||
| "peerDependenciesMeta": { | ||
| "oxlint": { | ||
| "optional": true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display package.json
if [ -f "package.json" ]; then
echo "=== package.json content ==="
cat -n package.json
else
echo "package.json not found"
fiRepository: eggjs/oxlint-config
Length of output: 1358
🏁 Script executed:
#!/bin/bash
# Check index.mjs
if [ -f "index.mjs" ]; then
echo "=== index.mjs content ==="
cat -n index.mjs
else
echo "index.mjs not found"
fiRepository: eggjs/oxlint-config
Length of output: 5865
🏁 Script executed:
#!/bin/bash
# Check .oxlintrc.json and README
echo "=== .oxlintrc.json content ==="
if [ -f ".oxlintrc.json" ]; then
cat .oxlintrc.json
else
echo ".oxlintrc.json not found"
fi
echo ""
echo "=== README.md (first 50 lines) ==="
if [ -f "README.md" ]; then
head -50 README.md
else
echo "README.md not found"
fiRepository: eggjs/oxlint-config
Length of output: 5701
Don't mark oxlint optional when the main export imports it.
index.mjs line 1 has a hard runtime import from oxlint, and the primary README examples show consuming via import oxlintConfig from '@eggjs/oxlint-config'. With the peer marked optional, oxlint won't be installed by default, causing MODULE_NOT_FOUND for users following the documented JS import pattern.
Proposed fix
"peerDependencies": {
"oxlint": ">=1.51.0"
},
- "peerDependenciesMeta": {
- "oxlint": {
- "optional": true
- }
- },
"devDependencies": {
"oxlint": "^1.51.0"
}If you want to keep supporting JSON-only consumers via the .oxlintrc.json export without requiring oxlint, document that the legacy JSON config is an alternative for projects that don't need the JS API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 36 - 42, The package currently marks the
peerDependency "oxlint" as optional while the package's runtime (index.mjs) does
a direct import of oxlint, which will cause MODULE_NOT_FOUND for JS consumers;
update package.json by removing the "optional": true entry under
"peerDependenciesMeta" for "oxlint" (or otherwise ensure "oxlint" is a required
peerDependency) so that installing this package will require oxlint to be
installed, and keep README notes about JSON-only usage if you still want to
support consumers that avoid the JS API.
1 participant
Breaking Change
oxlint >= 1.51.0 (bundled in vite-plus 0.1.1) no longer supports string paths in
extends. Config objects wrapped withdefineConfig()are now required.What changed
index.mjsthat exports the config as a JS object usingdefineConfig()from oxlintpackage.jsonexports to point toindex.mjsas the default entryoxlint >= 1.51.0as an optional peer dependency.oxlintrc.jsonis still available for direct CLI useUsage
Summary by CodeRabbit
New Features
Documentation
Chores