chore: replace ESLint and Prettier with oxlint and oxfmt#40
chore: replace ESLint and Prettier with oxlint and oxfmt#40Prashant-Surya merged 5 commits intomainfrom
Conversation
Migrates from ESLint + Prettier to the oxc toolchain (oxlint + oxfmt) for faster linting and formatting. Uses the automated migration tools (@oxlint/migrate and oxfmt --migrate=prettier) to preserve existing rule configuration and formatting settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaced ESLint/Prettier with oxlint/oxfmt (configs and scripts), added formatting/lint configs, adjusted package.json (deps, scripts, overrides), normalized CI YAML formatting, and applied widespread non-functional formatting edits across source, tests, docs, and configs. No runtime or API behavior changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
The `resolutions` field is Yarn-specific. Use `pnpm.overrides` instead, which is the correct equivalent for pnpm package manager. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/WorkItems/WorkLogs.ts (1)
55-59:⚠️ Potential issue | 🟠 MajorUse
delas the standard delete method name.This resource exposes
delete(...)instead of the standarddel(...). Please makedel(...)canonical (optionally keepdelete(...)as a compatibility alias).Suggested change
- async delete(workspaceSlug: string, projectId: string, workItemId: string, workLogId: string): Promise<void> { - return this.httpDelete( - `/workspaces/${workspaceSlug}/projects/${projectId}/work-items/${workItemId}/worklogs/${workLogId}/` - ); - } + async del(workspaceSlug: string, projectId: string, workItemId: string, workLogId: string): Promise<void> { + return this.httpDelete( + `/workspaces/${workspaceSlug}/projects/${projectId}/work-items/${workItemId}/worklogs/${workLogId}/` + ); + } + + /** `@deprecated` Use del(...) */ + async delete(workspaceSlug: string, projectId: string, workItemId: string, workLogId: string): Promise<void> { + return this.del(workspaceSlug, projectId, workItemId, workLogId); + }As per coding guidelines:
src/api/**/*.ts: Standard resource methods should be named:list,create,retrieve,update,del.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/WorkItems/WorkLogs.ts` around lines 55 - 59, The method on WorkLogs currently exposes delete(...) but must use the standard del(...) name; add a new async del(workspaceSlug: string, projectId: string, workItemId: string, workLogId: string): Promise<void> that calls the same this.httpDelete(...) path, and preserve the existing delete(...) as a thin compatibility alias that forwards to del(...). Update references/tests to use del if any.src/api/WorkItems/Attachments.ts (1)
81-85:⚠️ Potential issue | 🟠 MajorExpose
delas the standard delete method.
delete(...)deviates from the resource method naming convention. Please makedel(...)the canonical method (you can keepdelete(...)as a temporary alias for compatibility).Suggested change
- async delete(workspaceSlug: string, projectId: string, workItemId: string, attachmentId: string): Promise<void> { - return this.httpDelete( - `/workspaces/${workspaceSlug}/projects/${projectId}/work-items/${workItemId}/attachments/${attachmentId}/` - ); - } + async del(workspaceSlug: string, projectId: string, workItemId: string, attachmentId: string): Promise<void> { + return this.httpDelete( + `/workspaces/${workspaceSlug}/projects/${projectId}/work-items/${workItemId}/attachments/${attachmentId}/` + ); + } + + /** `@deprecated` Use del(...) */ + async delete(workspaceSlug: string, projectId: string, workItemId: string, attachmentId: string): Promise<void> { + return this.del(workspaceSlug, projectId, workItemId, attachmentId); + }As per coding guidelines:
src/api/**/*.ts: Standard resource methods should be named:list,create,retrieve,update,del.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/WorkItems/Attachments.ts` around lines 81 - 85, The method name delete(...) breaks the standard resource naming; add a new canonical del(workspaceSlug, projectId, workItemId, attachmentId): Promise<void> that performs the HTTP DELETE via this.httpDelete(...) and keep the existing delete(...) as a thin compatibility wrapper that simply calls and returns this.del(...); update any internal references to use del where appropriate and ensure both methods share the same signature (attachmentId param and Promise<void> return).
🧹 Nitpick comments (1)
.github/workflows/build-test.yaml (1)
21-23: Pin Node version in CI for deterministic runs.
actions/setup-node@v4is used withoutnode-version; runner-image changes can silently alter lint/build behavior. Pin to Node 20 to align with SDK engine expectations (>=20.0.0).Suggested workflow change
- name: Set up Node.js uses: actions/setup-node@v4 + with: + node-version: "20"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test.yaml around lines 21 - 23, The workflow uses actions/setup-node@v4 without pinning a Node version which can cause non-deterministic CI runs; update the GitHub Actions step that uses actions/setup-node@v4 to explicitly set node-version: '20' (or node-version: '20.x') so the runner uses Node 20 to match the SDK engine constraint (>=20.0.0) and ensure deterministic lint/build behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/WorkItems/Activities.ts`:
- Line 22: The parameter declaration params?: any in Activities.ts weakens type
safety; replace it with a concrete type (e.g., define and use an
ActivityListParams interface or type reflecting allowed query fields such as
page, pageSize, filter, sort) or, if the shape is genuinely unknown, use
params?: unknown and add runtime type guards/validation before use. Update the
function signature(s) referencing params and any callers to use the new
ActivityListParams (or perform validation where params is consumed) so the
codebase no longer uses any for list parameters.
---
Outside diff comments:
In `@src/api/WorkItems/Attachments.ts`:
- Around line 81-85: The method name delete(...) breaks the standard resource
naming; add a new canonical del(workspaceSlug, projectId, workItemId,
attachmentId): Promise<void> that performs the HTTP DELETE via
this.httpDelete(...) and keep the existing delete(...) as a thin compatibility
wrapper that simply calls and returns this.del(...); update any internal
references to use del where appropriate and ensure both methods share the same
signature (attachmentId param and Promise<void> return).
In `@src/api/WorkItems/WorkLogs.ts`:
- Around line 55-59: The method on WorkLogs currently exposes delete(...) but
must use the standard del(...) name; add a new async del(workspaceSlug: string,
projectId: string, workItemId: string, workLogId: string): Promise<void> that
calls the same this.httpDelete(...) path, and preserve the existing delete(...)
as a thin compatibility alias that forwards to del(...). Update references/tests
to use del if any.
---
Nitpick comments:
In @.github/workflows/build-test.yaml:
- Around line 21-23: The workflow uses actions/setup-node@v4 without pinning a
Node version which can cause non-deterministic CI runs; update the GitHub
Actions step that uses actions/setup-node@v4 to explicitly set node-version:
'20' (or node-version: '20.x') so the runner uses Node 20 to match the SDK
engine constraint (>=20.0.0) and ensure deterministic lint/build behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 777bfd60-f612-4279-8cc6-b833474711bb
📒 Files selected for processing (36)
.github/workflows/build-test.yaml.github/workflows/publish-node-sdk.yml.oxfmtrc.json.oxlintrc.json.prettierignore.prettierrcCLAUDE.mdeslint.config.mjsexamples/README.mdjest.config.jspackage.jsonsrc/Configuration.tssrc/api/AgentRuns/Activities.tssrc/api/AgentRuns/index.tssrc/api/Initiatives/Epics.tssrc/api/Initiatives/Labels.tssrc/api/Initiatives/Projects.tssrc/api/Initiatives/index.tssrc/api/Teamspaces/Members.tssrc/api/Teamspaces/Projects.tssrc/api/Teamspaces/index.tssrc/api/WorkItemProperties/Options.tssrc/api/WorkItems/Activities.tssrc/api/WorkItems/Attachments.tssrc/api/WorkItems/WorkLogs.tstests/e2e/project.test.tstests/unit/README.mdtests/unit/agent-runs/activities.test.tstests/unit/agent-runs/agent-runs.test.tstests/unit/initiative.test.tstests/unit/milestone.test.tstests/unit/sticky.test.tstests/unit/teamspace.test.tstests/unit/work-items/work-items.test.tstests/unit/workspace-features.test.tstsconfig.jest.json
💤 Files with no reviewable changes (11)
- src/api/Teamspaces/index.ts
- tests/unit/agent-runs/agent-runs.test.ts
- src/api/Initiatives/index.ts
- tests/unit/workspace-features.test.ts
- .prettierrc
- src/api/AgentRuns/index.ts
- .prettierignore
- tests/unit/initiative.test.ts
- src/api/AgentRuns/Activities.ts
- tests/unit/sticky.test.ts
- eslint.config.mjs
| projectId: string, | ||
| workItemId: string, | ||
| params?: any, | ||
| params?: any |
There was a problem hiding this comment.
Replace any in list params with an explicit type.
Line 22 uses params?: any, which weakens type safety and violates the repo TypeScript rule.
Proposed fix
- params?: any
+ params?: { limit?: number; offset?: number }As per coding guidelines: "src/**/*.ts: Avoid any types; use proper typing or unknown with type guards".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| params?: any | |
| params?: { limit?: number; offset?: number } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/WorkItems/Activities.ts` at line 22, The parameter declaration
params?: any in Activities.ts weakens type safety; replace it with a concrete
type (e.g., define and use an ActivityListParams interface or type reflecting
allowed query fields such as page, pageSize, filter, sort) or, if the shape is
genuinely unknown, use params?: unknown and add runtime type guards/validation
before use. Update the function signature(s) referencing params and any callers
to use the new ActivityListParams (or perform validation where params is
consumed) so the codebase no longer uses any for list parameters.
This repo is standalone and not part of a pnpm workspace catalog, so explicit version ranges are needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove flatted, minimatch@9, and ajv@6 overrides as none of these packages exist in the dependency tree. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
27-30: Consider using--deny-warningsto fail CI on lint warnings.The
check:lintscript currently allows warnings to pass. With 107 warnings noted in the PR, use oxlint's--deny-warningsflag to ensure warnings produce a non-zero exit code and fail the build.Suggested script update
- "check:lint": "oxlint", + "check:lint": "oxlint --deny-warnings",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 27 - 30, Update the lint check script so lint warnings cause CI failure: modify the "check:lint" npm script (in package.json scripts) to include oxlint's --deny-warnings flag (e.g., add --deny-warnings to the existing "check:lint" command) so oxlint exits non‑zero on warnings; ensure no other scripts (like "fix:lint") are changed so developers can still run fixes locally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 27-30: Update the lint check script so lint warnings cause CI
failure: modify the "check:lint" npm script (in package.json scripts) to include
oxlint's --deny-warnings flag (e.g., add --deny-warnings to the existing
"check:lint" command) so oxlint exits non‑zero on warnings; ensure no other
scripts (like "fix:lint") are changed so developers can still run fixes locally.
Summary
@oxlint/migrateandoxfmt --migrate=prettierto preserve existing rules and formatting settingseslint,@eslint/js,@typescript-eslint/*,typescript-eslint,globals,eslint-plugin-unused-imports) andprettierunused-imports/*rules with nativetypescript/no-unused-vars(oxlint built-in)Changes
.oxlintrc.json,.oxfmtrc.jsoneslint.config.mjs,.prettierrc,.prettierignorepackage.jsonscripts and devDependencies,CLAUDE.mdTest plan
pnpm check:lint— passes (107 warnings, 0 errors)pnpm check:format— passes afterpnpm fix:formatpnpm fix:lint— auto-fix workspnpm fix:format— formatting workspnpm build— builds successfullySummary by CodeRabbit
Chores
Documentation