feat: adopt load-oxfmt-config latest API and add cli parity mode#17
feat: adopt load-oxfmt-config latest API and add cli parity mode#17
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a new ChangesCLI Parity Mode Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 |
commit: |
There was a problem hiding this comment.
Pull request overview
Updates the plugin to the load-oxfmt-config@0.5 API and introduces a new cliParity preset/mode so ESLint-driven formatting can more closely mirror oxfmt CLI ignore + config behavior.
Changes:
- Refactor the worker to use
loadOxfmtConfigResult/isOxfmtIgnoredand add CLI-parity ignore orchestration. - Add
configs.cliParity, expand rule/schema/types for new CLI-parity options, and document usage in the README. - Add dedicated CLI-parity fixtures + tests; bump dependencies to
oxfmt@0.48.0andload-oxfmt-config@0.5.0.
Reviewed changes
Copilot reviewed 29 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| workers/oxfmt.mjs | Refactors config/ignore handling for load-oxfmt-config@0.5 and adds CLI-parity orchestration. |
| tests/worker.test.ts | Updates worker option typing/tests for new validation behavior. |
| tests/schema-parity.test.ts | Extends plugin-only option allowlist for schema parity checks. |
| tests/fixtures/config-loading/jsonc/.oxfmtrc.jsonc | Fixture formatting tweak (trailing comma removal). |
| tests/fixtures/config-loading/jsonc-with-editorconfig/.oxfmtrc.jsonc | Fixture formatting tweak (trailing comma removal). |
| tests/fixtures/config-loading/config-priority/.oxfmtrc.jsonc | Fixture formatting tweak (trailing comma removal). |
| tests/fixtures/cli-parity/src/unformatted.js | Adds CLI-parity fixture input file. |
| tests/fixtures/cli-parity/src/prettierignored.js | Adds .prettierignore-covered fixture. |
| tests/fixtures/cli-parity/src/ignored-by-custom.js | Adds custom ignore-path covered fixture. |
| tests/fixtures/cli-parity/pnpm-lock.yaml | Adds lockfile fixture for default ignore behavior. |
| tests/fixtures/cli-parity/package-lock.json | Adds lockfile fixture for default ignore behavior. |
| tests/fixtures/cli-parity/node_modules/a.ts | Adds node_modules fixture for default ignore behavior. |
| tests/fixtures/cli-parity/nested/packages/a/src/example.ts | Adds nested-config fixture file. |
| tests/fixtures/cli-parity/nested/packages/a/.oxfmtrc.json | Adds nested-config fixture config. |
| tests/fixtures/cli-parity/nested/.oxfmtrc.json | Adds root config for nested-config fixture. |
| tests/fixtures/cli-parity/ignores/custom.ignore | Adds custom ignore file used by ignorePath tests. |
| tests/fixtures/cli-parity/configs/project/src/example.ts | Adds config-dir-relative ignorePatterns fixture (non-ignored file). |
| tests/fixtures/cli-parity/configs/project/ignored-config/example.ts | Adds config-dir-relative ignorePatterns fixture (ignored file). |
| tests/fixtures/cli-parity/configs/project/.oxfmtrc.json | Adds fixture config with ignorePatterns relative to config dir. |
| tests/fixtures/cli-parity/.prettierignore | Adds .prettierignore fixture for parity tests. |
| tests/fixtures/cli-parity/.oxfmtrc.json | Adds root oxfmt config fixture for parity tests. |
| tests/fixtures/cli-parity/.gitignore | Adds .gitignore fixture for parity tests. |
| tests/eslint-plugin.test.ts | Switches to shared RuleOxfmtOptions type. |
| tests/configs.test.ts | Adds coverage for the new configs.cliParity preset. |
| tests/cli-parity.test.ts | New end-to-end tests validating CLI-parity ignore/config behavior. |
| src/types.ts | Centralizes RuleOxfmtOptions / WorkerFormatResult types and adds configs.cliParity typing. |
| src/schema.ts | Expands schema descriptions and adds CLI-parity-related plugin options. |
| src/rules/oxfmt.ts | Handles worker “ignored” responses by skipping difference reporting. |
| src/configs.ts | Adds and exports cliParity preset. |
| scripts/checkSchemaParity.ts | Updates plugin-only option allowlist for schema parity script. |
| README.md | Documents the new cliParity preset and CLI parity behavior/options. |
| pnpm-lock.yaml | Locks updated dependency versions (notably oxfmt and load-oxfmt-config). |
| package.json | Bumps load-oxfmt-config / oxfmt and dev tooling versions. |
| dts/rule-options.d.ts | Regenerates rule option types to include new plugin options. |
| .gitignore | Allows committing tests/fixtures/**/node_modules/ for parity fixtures. |
Files not reviewed (2)
- tests/fixtures/cli-parity/package-lock.json: Language not supported
- tests/fixtures/cli-parity/pnpm-lock.yaml: Language not supported
| /** @type {Map<string, Promise<string[]>>} */ | ||
| const ignorePathPatternsCache = new Map() | ||
|
|
| const fileMatcher = picomatch(override.files) | ||
| const matches = !!fileMatcher && fileMatcher(relativePath) | ||
|
|
||
| // Check if file is excluded | ||
| const excluded = | ||
| excludeFiles && excludeFiles.length > 0 | ||
| ? getCachedMatcher(excludeFiles)(relativePath) | ||
| : false | ||
| const excludeMatcher = override.excludeFiles?.length | ||
| ? picomatch(override.excludeFiles) | ||
| : undefined | ||
| const excluded = excludeMatcher ? excludeMatcher(relativePath) : false |
| it('should allow invoking the worker without an options object', () => { | ||
| const result = runWorker('test.js', 'const value = 1;', { | ||
| useConfig: false, | ||
| }) | ||
| expect(result).toBeTypeOf('object') | ||
| }) |
| function shouldIgnoreFile(relativePath, ignorePatterns) { | ||
| if (!ignorePatterns?.length) { | ||
| return false | ||
| } | ||
|
|
||
| // Check if file matches any ignore pattern | ||
| return getCachedMatcher(ignorePatterns)(relativePath) | ||
| const matcher = picomatch(ignorePatterns) | ||
| return !!matcher && matcher(relativePath) | ||
| } |
![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
🧹 Nitpick comments (5)
scripts/checkSchemaParity.ts (1)
9-16: ⚡ Quick win
PLUGIN_ONLY_TOP_LEVEL_OPTIONSis duplicated verbatim intests/schema-parity.test.ts.Both this script and the Vitest test define the same
Setindependently. The fact that both had to be updated in this PR is a sign that the next new plugin-only option will require a third touch in two separate files—and a mismatch will silently cause one check to pass while the other flags a false failure (or vice versa).Consider extracting it to a shared location (e.g.,
scripts/constants.ts) and importing from both:♻️ Proposed refactor
// scripts/constants.ts (new file) +export const PLUGIN_ONLY_TOP_LEVEL_OPTIONS = new Set([ + 'configPath', + 'disableNestedConfig', + 'ignorePath', + 'respectOxfmtDefaultIgnores', + 'useConfig', + 'withNodeModules', +])// scripts/checkSchemaParity.ts -const PLUGIN_ONLY_TOP_LEVEL_OPTIONS = new Set([ - 'configPath', - 'disableNestedConfig', - 'ignorePath', - 'respectOxfmtDefaultIgnores', - 'useConfig', - 'withNodeModules', -]) +import { PLUGIN_ONLY_TOP_LEVEL_OPTIONS } from './constants'// tests/schema-parity.test.ts -const PLUGIN_ONLY_TOP_LEVEL_OPTIONS = new Set([ - 'configPath', - 'disableNestedConfig', - 'ignorePath', - 'respectOxfmtDefaultIgnores', - 'useConfig', - 'withNodeModules', -]) +import { PLUGIN_ONLY_TOP_LEVEL_OPTIONS } from '../scripts/constants'🤖 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 `@scripts/checkSchemaParity.ts` around lines 9 - 16, PLUGIN_ONLY_TOP_LEVEL_OPTIONS is duplicated in scripts/checkSchemaParity.ts and tests/schema-parity.test.ts; extract the Set into a single exported constant (e.g., export const PLUGIN_ONLY_TOP_LEVEL_OPTIONS) in a new shared module like scripts/constants.ts and update both scripts/checkSchemaParity.ts and tests/schema-parity.test.ts to import that constant instead of redefining it so future changes only need one edit.workers/oxfmt.mjs (3)
30-40: 💤 Low value
editorconfiganduseCacheare routed through but never validated.Both keys are listed in
PLUGIN_ONLY_OPTIONSand forwarded toloadOxfmtConfigResult/isOxfmtIgnored, butvalidatePluginOptionsskips them. If a user passes a wrong-shaped value the failure surfaces deep inside the helper library with a less actionable message. Consider a lightweight type check (useCache: boolean,editorconfig: boolean | object) for parity with the other validations — or, conversely, document why these are intentionally pass-through. Also,useCachedoes not appear in the public schema/README — confirm it's meant to be a hidden option.Also applies to: 91-146
🤖 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 `@workers/oxfmt.mjs` around lines 30 - 40, PLUGIN_ONLY_OPTIONS includes editorconfig and useCache but validatePluginOptions does not type-check them, causing poor errors downstream; update validatePluginOptions to add lightweight checks ensuring useCache is a boolean and editorconfig is either boolean or plain object (or explicitly allow null/undefined if intended), and adjust any calls into loadOxfmtConfigResult / isOxfmtIgnored to rely on the validated shapes (or add a comment documenting intentional pass-through); also verify whether useCache is intended to be public and, if not, document it as a hidden option in the README/schema.
274-280: 💤 Low value
cwdmay beundefinedwhendisableNestedConfig: true.
cwd: pluginOptions.disableNestedConfig ? cwd : dirname(filename)passesundefineddirectly toloadOxfmtConfigResultif the caller did not supplycwd. Through the rule path that's fine because the rule always injectscontext.cwd, but the worker is also a public-ish entry point (used directly fromtests/cli-parity.test.tsandtests/worker.test.ts). Acwd ?? process.cwd()(or explicit error) would make the contract less surprising.🤖 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 `@workers/oxfmt.mjs` around lines 274 - 280, The call to loadOxfmtConfigResult can pass undefined for cwd when pluginOptions.disableNestedConfig is true and the worker caller didn't supply cwd; update the logic around useConfig to ensure cwd defaults to process.cwd() before passing into loadOxfmtConfigResult (e.g. compute a resolvedCwd = pluginOptions.disableNestedConfig ? (cwd ?? process.cwd()) : dirname(filename) or similar) so loadOxfmtConfigResult always receives a defined cwd; update references to cwd in that call to use resolvedCwd and keep the same behavior when disableNestedConfig is false by still using dirname(filename).
148-149: ⚡ Quick win
ignorePathPatternsCacheis unbounded.The module-level
Mapgrows indefinitely across worker lifetime — every distinctignorePath(or array combination) adds a permanent entry. The repository guideline requires module-level caches to enforce aMAX_CACHE_SIZEwith FIFO/oldest-entry eviction. In practice this is unlikely to balloon in normal use, but long-running editor integrations (LSP-backed ESLint server) can rotate through many cwd/ignorePath combinations.🛠️ Suggested approach
+const MAX_CACHE_SIZE = 100 + /** `@type` {Map<string, Promise<string[]>>} */ const ignorePathPatternsCache = new Map() @@ async function loadIgnorePathPatterns(ignorePath) { const paths = Array.isArray(ignorePath) ? ignorePath : [ignorePath] const cacheKey = paths.join('\0') const cached = ignorePathPatternsCache.get(cacheKey) if (cached) { + // Refresh recency for FIFO-on-insert eviction return cached } @@ - ignorePathPatternsCache.set(cacheKey, task) + if (ignorePathPatternsCache.size >= MAX_CACHE_SIZE) { + const oldest = ignorePathPatternsCache.keys().next().value + if (oldest !== undefined) ignorePathPatternsCache.delete(oldest) + } + ignorePathPatternsCache.set(cacheKey, task)As per coding guidelines: "Worker module-level caches must use FIFO/oldest-entry eviction with MAX_CACHE_SIZE limit".
Also applies to: 336-365
🤖 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 `@workers/oxfmt.mjs` around lines 148 - 149, The module-level Map ignorePathPatternsCache is unbounded; add a MAX_CACHE_SIZE constant and change how entries are inserted so that when adding a new key to ignorePathPatternsCache (which must remain a Map<string, Promise<string[]>>), you check size > MAX_CACHE_SIZE and evict the oldest entry (FIFO) before inserting the new one; ensure all places that set or read ignorePathPatternsCache (including the code around the other uses referenced later) use this bounded-insert logic so the cache never grows past MAX_CACHE_SIZE.tests/cli-parity.test.ts (1)
77-94: ⚡ Quick win
runWorkerdirect invocation bypasses ESLint's file discovery.Unlike the other cases that go through
lintFile, this path calls the worker directly. That's fine for asserting worker behavior, but it means the test doesn't actually validate that thecliParitypreset (with itsfiles/ignores) letsnode_modulesthrough end-to-end whenwithNodeModules: true. Consider mirroring the other cases vialintFile(..., { withNodeModules: true })to cover the rule-level integration as well — particularly because the config'signores: ['!**/node_modules/**']re-include is tricky and easy to regress.🤖 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 `@tests/cli-parity.test.ts` around lines 77 - 94, The test currently calls runWorker(...) directly which bypasses ESLint/cli file-discovery and ignores handling; change the invocation to use lintFile(filePath, { cwd: FIXTURE_CWD, respectOxfmtDefaultIgnores: true, useConfig: true, withNodeModules: true }) (or the existing lintFile helper used in other tests) so the test exercises the cliParity preset's files/ignores re-include for node_modules end-to-end; keep the same assertions (expect(result.ignored)... and expect(result.code)...), and await the lintFile call if it returns a Promise to preserve async behavior.
🤖 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 `@tests/cli-parity.test.ts`:
- Around line 109-112: The test "should ignore files matched by .gitignore"
fails because the lintFile helper reads the fixture file (gitignored.js) but
that fixture is not committed due to a .gitignore rule; ensure the fixture file
is present in the repo for CI by either adding an allowlist entry for
gitignored.js in the repo-level .gitignore so the file is tracked while
remaining locally ignored by the fixture's .gitignore, or force-add the fixture
(git add -f) and document the requirement; update the repository so the lintFile
helper can read the fixture and the test in cli-parity.test.ts passes.
In `@tests/worker.test.ts`:
- Around line 17-22: The test title and body disagree: the title says "without
an options object" but the test calls runWorker('test.js', 'const value = 1;', {
useConfig: false }); fix by either removing the third argument to actually test
the no-options path (call runWorker('test.js', 'const value = 1;') and keep the
existing assertion) or update the test title to accurately describe the case
(e.g., "should accept minimal options without `cwd`" or "should accept an
options object with only `useConfig`"), and ensure the assertion remains
expect(result).toBeTypeOf('object'); reference runWorker invocation and the
options object passed in the test to locate the change.
In `@workers/oxfmt.mjs`:
- Around line 274-300: The code path under useConfig currently drops
inlineFormatOptions.overrides; change it to read rule-level overrides from
inlineFormatOptions (e.g., extract ruleOverrides from inlineFormatOptions as
done in the else branch) and merge them with the config overrides returned by
loadOxfmtConfigResult so config-level overrides come first and rule-level
overrides are appended (e.g., effectiveOverrides =
Array.isArray(configOverrides)? [...configOverrides,
...(Array.isArray(ruleOverrides)? ruleOverrides:[])] :
Array.isArray(ruleOverrides)? ruleOverrides : undefined), preserve
overrideBaseDir assignment from loaded.dirname, and ensure finalOptions still
spreads loadedConfig then inline options; also add a unit/integration test that
loads a config with an override and supplies an inline rule-level override that
should take precedence when keys conflict to verify rule-level overrides are
applied on top of config-level overrides.
---
Nitpick comments:
In `@scripts/checkSchemaParity.ts`:
- Around line 9-16: PLUGIN_ONLY_TOP_LEVEL_OPTIONS is duplicated in
scripts/checkSchemaParity.ts and tests/schema-parity.test.ts; extract the Set
into a single exported constant (e.g., export const
PLUGIN_ONLY_TOP_LEVEL_OPTIONS) in a new shared module like scripts/constants.ts
and update both scripts/checkSchemaParity.ts and tests/schema-parity.test.ts to
import that constant instead of redefining it so future changes only need one
edit.
In `@tests/cli-parity.test.ts`:
- Around line 77-94: The test currently calls runWorker(...) directly which
bypasses ESLint/cli file-discovery and ignores handling; change the invocation
to use lintFile(filePath, { cwd: FIXTURE_CWD, respectOxfmtDefaultIgnores: true,
useConfig: true, withNodeModules: true }) (or the existing lintFile helper used
in other tests) so the test exercises the cliParity preset's files/ignores
re-include for node_modules end-to-end; keep the same assertions
(expect(result.ignored)... and expect(result.code)...), and await the lintFile
call if it returns a Promise to preserve async behavior.
In `@workers/oxfmt.mjs`:
- Around line 30-40: PLUGIN_ONLY_OPTIONS includes editorconfig and useCache but
validatePluginOptions does not type-check them, causing poor errors downstream;
update validatePluginOptions to add lightweight checks ensuring useCache is a
boolean and editorconfig is either boolean or plain object (or explicitly allow
null/undefined if intended), and adjust any calls into loadOxfmtConfigResult /
isOxfmtIgnored to rely on the validated shapes (or add a comment documenting
intentional pass-through); also verify whether useCache is intended to be public
and, if not, document it as a hidden option in the README/schema.
- Around line 274-280: The call to loadOxfmtConfigResult can pass undefined for
cwd when pluginOptions.disableNestedConfig is true and the worker caller didn't
supply cwd; update the logic around useConfig to ensure cwd defaults to
process.cwd() before passing into loadOxfmtConfigResult (e.g. compute a
resolvedCwd = pluginOptions.disableNestedConfig ? (cwd ?? process.cwd()) :
dirname(filename) or similar) so loadOxfmtConfigResult always receives a defined
cwd; update references to cwd in that call to use resolvedCwd and keep the same
behavior when disableNestedConfig is false by still using dirname(filename).
- Around line 148-149: The module-level Map ignorePathPatternsCache is
unbounded; add a MAX_CACHE_SIZE constant and change how entries are inserted so
that when adding a new key to ignorePathPatternsCache (which must remain a
Map<string, Promise<string[]>>), you check size > MAX_CACHE_SIZE and evict the
oldest entry (FIFO) before inserting the new one; ensure all places that set or
read ignorePathPatternsCache (including the code around the other uses
referenced later) use this bounded-insert logic so the cache never grows past
MAX_CACHE_SIZE.
🪄 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: cf46ec95-1c6a-4a67-a913-8789b4768524
⛔ Files ignored due to path filters (4)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/fixtures/cli-parity/node_modules/a.tsis excluded by!**/node_modules/**tests/fixtures/cli-parity/package-lock.jsonis excluded by!**/package-lock.jsontests/fixtures/cli-parity/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.gitignoreREADME.mddts/rule-options.d.tspackage.jsonscripts/checkSchemaParity.tssrc/configs.tssrc/rules/oxfmt.tssrc/schema.tssrc/types.tstests/cli-parity.test.tstests/configs.test.tstests/eslint-plugin.test.tstests/fixtures/cli-parity/.gitignoretests/fixtures/cli-parity/.oxfmtrc.jsontests/fixtures/cli-parity/.prettierignoretests/fixtures/cli-parity/configs/project/.oxfmtrc.jsontests/fixtures/cli-parity/configs/project/ignored-config/example.tstests/fixtures/cli-parity/configs/project/src/example.tstests/fixtures/cli-parity/ignores/custom.ignoretests/fixtures/cli-parity/nested/.oxfmtrc.jsontests/fixtures/cli-parity/nested/packages/a/.oxfmtrc.jsontests/fixtures/cli-parity/nested/packages/a/src/example.tstests/fixtures/cli-parity/src/ignored-by-custom.jstests/fixtures/cli-parity/src/prettierignored.jstests/fixtures/cli-parity/src/unformatted.jstests/fixtures/config-loading/config-priority/.oxfmtrc.jsonctests/fixtures/config-loading/jsonc-with-editorconfig/.oxfmtrc.jsonctests/fixtures/config-loading/jsonc/.oxfmtrc.jsonctests/schema-parity.test.tstests/worker.test.tsworkers/oxfmt.mjs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 36 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- tests/fixtures/cli-parity/package-lock.json: Language not supported
- tests/fixtures/cli-parity/pnpm-lock.yaml: Language not supported
| if (pluginOptions.respectOxfmtDefaultIgnores !== false && useConfig) { | ||
| /** @type {import('load-oxfmt-config').IsOxfmtIgnoredOptions & {useConfig?: boolean}} */ | ||
| const ignoredOptions = { | ||
| configPath: pluginOptions.configPath, | ||
| cwd, | ||
| disableNestedConfig: pluginOptions.disableNestedConfig, | ||
| filepath: filename, | ||
| ignorePath: normalizedIgnorePath, |
| async function loadIgnorePathPatterns(ignorePath) { | ||
| const paths = Array.isArray(ignorePath) ? ignorePath : [ignorePath] | ||
| const cacheKey = paths.join('\0') | ||
| const cached = ignorePathPatternsCache.get(cacheKey) | ||
| if (cached) { | ||
| return cached | ||
| } | ||
|
|
| it('should allow invoking the worker without an options object', () => { | ||
| const result = runWorker('test.js', 'const value = 1;', { | ||
| useConfig: false, | ||
| }) | ||
| expect(result).toBeTypeOf('object') |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 38 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- tests/fixtures/cli-parity/package-lock.json: Language not supported
- tests/fixtures/cli-parity/pnpm-lock.yaml: Language not supported
| function validatePluginOptions(pluginOptions) { | ||
| if (pluginOptions.cwd != null && typeof pluginOptions.cwd !== 'string') { | ||
| throw new TypeError( | ||
| 'oxfmt worker requires "cwd" to be a string when provided.', | ||
| ) | ||
| } | ||
|
|
||
| // Check if file matches any ignore pattern | ||
| return getCachedMatcher(ignorePatterns)(relativePath) | ||
| } | ||
|
|
||
| runAsWorker( | ||
| async ( | ||
| /** | ||
| * @type {string} filename | ||
| */ | ||
| filename, | ||
| /** | ||
| * @type {string} source text | ||
| */ | ||
| sourceText, | ||
| /** | ||
| * @type {Options} format options | ||
| */ | ||
| options, | ||
| ) => { | ||
| const { | ||
| configPath, | ||
| cwd, | ||
| formatOptions, | ||
| ignorePatterns, | ||
| overrides, | ||
| useConfig, | ||
| } = getWorkerOptions(options) | ||
|
|
||
| const { | ||
| configDir, | ||
| formatOptions: baseFormatOptions, | ||
| ignorePatterns: baseIgnorePatterns, | ||
| overrides: baseOverrides, | ||
| } = await resolveBaseOptions( | ||
| filename, | ||
| cwd, | ||
| configPath, | ||
| useConfig, | ||
| formatOptions, | ||
| if ( | ||
| pluginOptions.configPath != null && | ||
| typeof pluginOptions.configPath !== 'string' | ||
| ) { | ||
| throw new TypeError( | ||
| 'oxfmt worker requires "configPath" to be a string when provided.', | ||
| ) | ||
| const effectiveIgnorePatterns = ignorePatterns ?? baseIgnorePatterns | ||
| const ignoreBase = ignorePatterns == null ? configDir : cwd | ||
| const ignoreRelativePath = effectiveIgnorePatterns?.length | ||
| ? getRelativePath(ignoreBase, filename) | ||
| : undefined | ||
|
|
||
| if ( | ||
| ignoreRelativePath && | ||
| shouldIgnoreFile(ignoreRelativePath, effectiveIgnorePatterns) | ||
| ) { | ||
| return { code: sourceText } | ||
| } | ||
|
|
||
| const effectiveOverrides = useConfig ? baseOverrides : overrides | ||
| const overrideBase = useConfig ? configDir : cwd | ||
| const overrideRelativePath = effectiveOverrides?.length | ||
| ? getRelativePath(overrideBase, filename) | ||
| : undefined | ||
| const mergedOptionsCacheKey = getMergedOptionsCacheKey( | ||
| baseFormatOptions, | ||
| overrideRelativePath, | ||
| effectiveOverrides, | ||
| } | ||
| if ( | ||
| pluginOptions.ignorePath != null && | ||
| typeof pluginOptions.ignorePath !== 'string' && | ||
| !isStringArray(pluginOptions.ignorePath) | ||
| ) { | ||
| throw new TypeError( | ||
| 'oxfmt worker requires "ignorePath" to be a string or string array when provided.', | ||
| ) | ||
| } | ||
| if ( | ||
| pluginOptions.useConfig != null && | ||
| typeof pluginOptions.useConfig !== 'boolean' | ||
| ) { | ||
| throw new TypeError( | ||
| 'oxfmt worker requires "useConfig" to be a boolean when provided.', | ||
| ) | ||
| } | ||
| if ( | ||
| pluginOptions.withNodeModules != null && | ||
| typeof pluginOptions.withNodeModules !== 'boolean' | ||
| ) { | ||
| throw new TypeError( | ||
| 'oxfmt worker requires "withNodeModules" to be a boolean when provided.', |
|
|
||
| ## Behavior That Is Easy To Break | ||
|
|
||
| - In [workers/oxfmt.mjs](workers/oxfmt.mjs), when `useConfig` is true, `overrides` come from loaded config; rule-level `overrides` are ignored by design. |
| import type { format } from 'oxfmt' | ||
| import type { RuleOxfmtOptions } from '../src/types' | ||
|
|
||
| type FormatResult = Awaited<ReturnType<typeof format>> | ||
| type WorkerOptions = FormatConfig & LoadOxfmtConfigOptions | ||
|
|
||
| const runWorker = createSyncFn(join(dirWorkers, 'oxfmt.mjs')) as unknown as ( | ||
| filename: string, | ||
| sourceText: string, | ||
| options?: WorkerOptions, | ||
| options?: RuleOxfmtOptions, | ||
| ) => FormatResult |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@workers/oxfmt.mjs`:
- Around line 148-167: The code currently merges rule-level overrides into
effectiveOverrides when useConfig is true (see loadOxfmtConfigResult,
configOverrides, ruleOverrides, inlineFormatOptions, and effectiveOverrides),
which contradicts the guideline that rule-level overrides must be ignored when
useConfig is true; update the useConfig branch to set effectiveOverrides
strictly from the loaded config (e.g., effectiveOverrides = configOverrides ??
[]) and do not append or reference ruleOverrides there, while keeping
finalOptions built from loadedConfig and inline options as before.
- Around line 110-123: The code calls isOxfmtIgnored and references
loadOxfmtConfigResult/IsOxfmtIgnoredOptions which are not exported by
load-oxfmt-config@0.4.1; either upgrade the dependency to a version that exports
those symbols, or change this file to use the 0.4.1 API (loadOxfmtConfig and
resolveOxfmtrcPath). If you choose the latter, remove the JSDoc type
IsOxfmtIgnoredOptions, replace the isOxfmtIgnored call with logic that loads the
config via loadOxfmtConfig/resolveOxfmtrcPath and derives ignore patterns (or
checks the ignorePath) using the pluginOptions (configPath, cwd,
disableNestedConfig, filepath, ignorePath, useCache, withNodeModules,
useConfig), and store results in the existing ignored variable so subsequent
code uses the same shape; otherwise bump the package version to one that exports
isOxfmtIgnored/loadOxfmtConfigResult and update imports accordingly.
🪄 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: 9cd7b7dd-df26-4b3f-b7d3-400ff2a28db8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/copilot-instructions.mdAGENTS.mdREADME.mdpackage.jsontests/cli-parity.test.tstests/eslint-plugin.test.tstests/fixtures/cli-parity/custom.ignoretests/fixtures/cli-parity/ignores/custom.ignoretests/fixtures/cli-parity/src/gitignored.jstests/worker.test.tsworkers/oxfmt.mjs
💤 Files with no reviewable changes (1)
- .github/copilot-instructions.md
✅ Files skipped from review due to trivial changes (4)
- tests/fixtures/cli-parity/ignores/custom.ignore
- AGENTS.md
- tests/fixtures/cli-parity/src/gitignored.js
- tests/eslint-plugin.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- package.json
- README.md
- tests/worker.test.ts
- tests/cli-parity.test.ts
| if (pluginOptions.respectOxfmtDefaultIgnores !== false && cwd) { | ||
| /** @type {import('load-oxfmt-config').IsOxfmtIgnoredOptions & {includeConfigIgnorePatterns?: boolean, loadConfigForIgnorePatterns?: boolean}} */ | ||
| const ignoredOptions = { | ||
| configPath: pluginOptions.configPath, | ||
| cwd, | ||
| disableNestedConfig: pluginOptions.disableNestedConfig, | ||
| filepath: filename, | ||
| ignorePath: pluginOptions.ignorePath, | ||
| includeConfigIgnorePatterns: useConfig, | ||
| loadConfigForIgnorePatterns: useConfig, | ||
| useCache: pluginOptions.useCache, | ||
| withNodeModules: pluginOptions.withNodeModules, | ||
| } | ||
| const ignored = await isOxfmtIgnored(ignoredOptions) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
load-oxfmt-config 0.5 isOxfmtIgnored options includeConfigIgnorePatterns loadConfigForIgnorePatterns IsOxfmtIgnoredOptions
💡 Result:
load-oxfmt-config is a Node.js package (latest version appears to be 0.3.1 based on CDN data[1], no explicit 0.5 found) that loads and resolves oxfmt configuration files (.oxfmtrc.json,.oxfmtrc.jsonc, oxfmt.config.ts) with auto-discovery, caching, TypeScript support, and merges supported.editorconfig settings into OxfmtOptions[2][3][4]. Its main API is loadOxfmtConfig(options?), which returns Promise or {} if no config found. Options include cwd, configPath (explicit path), useCache, and editorconfig (true/false/EditorconfigOption with onlyCwd, cwd)[2][4]. resolveOxfmtrcPath(cwd, configPath?) resolves config path[4].
Citations:
- 1: https://www.jsdelivr.com/package/npm/load-oxfmt-config
- 2: https://npmx.dev/package/load-oxfmt-config
- 3: https://github.com/ntnyq/load-oxfmt-config
- 4: https://github.com/ntnyq/load-oxfmt-config/
🏁 Script executed:
#!/bin/bash
# Inspect the installed load-oxfmt-config typings/source to confirm the option names exist.
fd -t f -p 'node_modules/load-oxfmt-config' 2>/dev/null | head -50
fd -t f 'package.json' -p 'load-oxfmt-config' 2>/dev/null | xargs -I{} cat {} 2>/dev/null | head -100
rg -nP '\b(isOxfmtIgnored|IsOxfmtIgnoredOptions|includeConfigIgnorePatterns|loadConfigForIgnorePatterns)\b' -C2Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 1329
🏁 Script executed:
# Check the package.json to see which version of load-oxfmt-config is being used
cat package.json | grep -A5 'load-oxfmt-config'Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 223
🏁 Script executed:
# Try to find and inspect the load-oxfmt-config package type definitions
fd -p 'load-oxfmt-config' -t d | head -5Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Search for any TypeScript definition files or index files that might expose the API
find . -path '*/load-oxfmt-config/*' -name '*.d.ts' -o -path '*/load-oxfmt-config/*' -name 'index.*' 2>/dev/null | head -20Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 279
🏁 Script executed:
# Look for any documentation or test files that might show usage of isOxfmtIgnored
rg 'isOxfmtIgnored' -A5 -B2 --type js --type ts --type mjs 2>/dev/null | head -80Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Read the TypeScript definition file to inspect IsOxfmtIgnoredOptions
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mtsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 3421
🏁 Script executed:
# Also check if there's a package.json in the load-oxfmt-config directory for version confirmation
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/package.json | grep -E '"version"|"name"'Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 143
🏁 Script executed:
# Search for any isOxfmtIgnored function signature in the codebase that might show expected options
rg 'function isOxfmtIgnored|export.*isOxfmtIgnored' -A20 2>/dev/null | head -100Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check the actual source file (index.mjs) to see what's actually exported
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.mjs | head -150Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 5484
🏁 Script executed:
# Search for isOxfmtIgnored in the entire package
rg 'isOxfmtIgnored' ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/ 2>/dev/nullRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check if there's a different version of the package or if it's in a different location
find ./node_modules -path '*load-oxfmt-config*' -name '*.mjs' -o -path '*load-oxfmt-config*' -name '*.d.ts' 2>/dev/null | sortRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 173
🏁 Script executed:
# Get the full TypeScript definition file - read more lines
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mts | wc -lRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 73
🏁 Script executed:
# Read the entire TypeScript definition file
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mtsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 3421
🏁 Script executed:
# Check if isOxfmtIgnored might be exported from a different export or has a different name
rg 'export' ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mtsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 203
🏁 Script executed:
# Let me check the complete imports in workers/oxfmt.mjs
head -20 workers/oxfmt.mjsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 956
🏁 Script executed:
# Check if isOxfmtIgnored might be in a different package or if there's a local implementation
rg 'isOxfmtIgnored' . --type js --type mjs --type ts 2>/dev/null | head -30Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check the package.json for all relevant dependencies
cat package.json | grep -A20 '"dependencies"' | grep -B20 '"devDependencies"'Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 243
🏁 Script executed:
# Check if there's a GitHub repo for load-oxfmt-config to find the actual API
rg 'isOxfmtIgnored|IsOxfmtIgnoredOptions' . --type ts --type tsx --type js --type mjs -B3 -A10 2>/dev/null | head -100Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Look at the entire workers/oxfmt.mjs file to understand the context
cat workers/oxfmt.mjs | head -150Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 4928
🏁 Script executed:
# Search for where IsOxfmtIgnoredResult is used or defined
rg 'IsOxfmtIgnoredResult|IsOxfmtIgnoredOptions' . --type ts --type tsx --type js --type mjs 2>/dev/nullRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Let me look at the rest of workers/oxfmt.mjs to see how isOxfmtIgnored is used
tail -100 workers/oxfmt.mjsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 3027
🏁 Script executed:
# Check if there's a pnpm-lock.yaml or yarn.lock to see the actual resolved version
fd 'pnpm-lock.yaml|yarn.lock|package-lock.json' -t f 2>/dev/null | xargs -I {} sh -c 'echo "File: {}"; wc -l {}' 2>/dev/nullRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 296
🏁 Script executed:
# Look at load-oxfmt-config version info - maybe there's a newer version that has these exports
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/package.json | head -20Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 565
🏁 Script executed:
# Search the entire dist directory of load-oxfmt-config for isOxfmtIgnored
find ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist -type f -exec grep -l 'isOxfmtIgnored' {} \;Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check if isOxfmtIgnored might be defined elsewhere in the package
find ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config -type f \( -name '*.d.ts' -o -name '*.d.mts' -o -name '*.d.cts' \) -exec grep -l 'isOxfmtIgnored' {} \;Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Let me check the pnpm-lock.yaml to see what the actual resolved version and exports are
rg 'load-oxfmt-config' pnpm-lock.yaml -A20 | head -60Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 2899
🏁 Script executed:
# Check what version is actually in node_modules
find ./node_modules/.pnpm -maxdepth 1 -type d -name 'load-oxfmt-config*' | sortRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 127
🏁 Script executed:
# Let me check the 0.7.0 version if it exists
ls -la ./node_modules/.pnpm/load-oxfmt-config@0.7.0_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/ 2>/dev/null || echo "0.7.0 not found in that location"Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 102
🏁 Script executed:
# Check the actual index.d.mts file in 0.7.0 if it exists
cat ./node_modules/.pnpm/load-oxfmt-config@0.7.0_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mts 2>/dev/null || echo "File not found"Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 84
🏁 Script executed:
# Check the full exports from the 0.4.1 version
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.mjs | tail -50Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 2933
🏁 Script executed:
# Search for isOxfmtIgnored in the entire mjs file
grep -n 'isOxfmtIgnored\|export' ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.mjs | tail -30Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 121
🏁 Script executed:
# Let's run pnpm install to see what happens
cd . && pnpm list load-oxfmt-config 2>&1 | head -20Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# The package.json specifies ^0.7.0 but pnpm-lock shows 0.4.1 is installed.
# Let me check if there's a 0.7.0 export we're missing
find ./node_modules -path '*load-oxfmt-config*' -name 'index.*' -type f 2>/dev/nullRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 279
🏁 Script executed:
# Check if isOxfmtIgnored might be defined in a sub-export or side module
find ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config -type f \( -name '*.mjs' -o -name '*.js' -o -name '*.d.ts' -o -name '*.d.mts' \) | xargs grep -l 'isOxfmtIgnored' 2>/dev/nullRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 53
🏁 Script executed:
# Let me verify the actual node_modules structure - pnpm might be using symlinks
ls -la ./node_modules/load-oxfmt-config/ 2>/dev/null || echo "No direct link"Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 84
🏁 Script executed:
# Check the actual package.json in node_modules to see exports
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/package.json | grep -A20 '"exports"'Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 574
🏁 Script executed:
# The import says isOxfmtIgnored but 0.4.1 doesn't export it.
# Let me check if isOxfmtIgnored is defined locally in the repository
rg 'function isOxfmtIgnored|const isOxfmtIgnored|export.*isOxfmtIgnored' . --type js --type mjs --type ts 2>/dev/null | grep -v node_modulesRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check if there's perhaps a newer version available or if loadOxfmtConfigResult exists
rg 'loadOxfmtConfigResult' . --type js --type mjs --type ts 2>/dev/null | grep -v node_modulesRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Let me check the GitHub repo directly for load-oxfmt-config to see what version has these exports
web_search load-oxfmt-config github isOxfmtIgnored loadOxfmtConfigResult APIRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 120
🏁 Script executed:
# Let me check the actual current state - maybe these are defined elsewhere
grep -r 'isOxfmtIgnored' . --include='*.mjs' --include='*.js' --include='*.ts' 2>/dev/null | grep -v node_modules | head -20Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 240
🏁 Script executed:
# Look at the full type definitions in the d.mts file again - maybe IsOxfmtIgnoredOptions and IsOxfmtIgnoredResult are defined
cat ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mts | grep -i 'IsOxfmtIgnored'Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check if maybe load-oxfmt-config was recently updated and the docs mention these functions
web_search 'load-oxfmt-config' 'isOxfmtIgnored' github ntnyqRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 120
🏁 Script executed:
# Let me check if there's a newer version of load-oxfmt-config available on npm
cat package.json | grep -E '"name"|"version"' | head -5Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 145
🏁 Script executed:
# Try checking if maybe the code should be using loadOxfmtConfig instead of isOxfmtIgnored
rg 'loadOxfmtConfig' ./node_modules/.pnpm/load-oxfmt-config@0.4.1_oxfmt@0.48.0/node_modules/load-oxfmt-config/dist/index.d.mtsRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 340
🏁 Script executed:
# The installed version is 0.4.1, but package.json specifies ^0.7.0.
# Let me check if there are any test files that might show when isOxfmtIgnored was introduced
find . -path './tests/**' -name '*.test.*' -o -path './tests/**' -name '*.spec.*' 2>/dev/null | head -10Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 471
🏁 Script executed:
# Check if there's a CHANGELOG or git history that might tell us when isOxfmtIgnored was added
find . -maxdepth 2 -type f \( -name 'CHANGELOG*' -o -name 'HISTORY*' \) 2>/dev/null | xargs cat 2>/dev/null | head -100Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Let me check the git log to see recent changes
git log --oneline -20 2>/dev/null || echo "No git history available"Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 112
🏁 Script executed:
# Check the git diff or status to see if this is a pending change
git diff HEAD workers/oxfmt.mjs 2>/dev/null | head -50 || git status workers/oxfmt.mjs 2>/dev/null || echo "No git changes"Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check the cli-parity test to understand what isOxfmtIgnored should do
cat ./tests/cli-parity.test.ts | head -100Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 2916
🏁 Script executed:
# Let me check if maybe there's a local type declaration or if the package needs to be updated
rg 'IsOxfmtIgnoredOptions|IsOxfmtIgnoredResult' . --type ts --type tsx --type mjs --type js 2>/dev/null | grep -v node_modules | head -20Repository: ntnyq/eslint-plugin-oxfmt
Length of output: 51
🏁 Script executed:
# Check if pnpm needs to be run to install the correct version
head -20 pnpm-lock.yamlRepository: ntnyq/eslint-plugin-oxfmt
Length of output: 444
isOxfmtIgnored and loadOxfmtConfigResult are not exported from load-oxfmt-config@0.4.1.
The code imports isOxfmtIgnored and loadOxfmtConfigResult from load-oxfmt-config, but the installed version (0.4.1) only exports loadOxfmtConfig and resolveOxfmtrcPath. The JSDoc cast on line 111 references IsOxfmtIgnoredOptions, which also doesn't exist in the package. This will cause a runtime error: "isOxfmtIgnored is not exported from 'load-oxfmt-config'". Either upgrade to a version that exports these functions or use the correct API from 0.4.1.
🤖 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 `@workers/oxfmt.mjs` around lines 110 - 123, The code calls isOxfmtIgnored and
references loadOxfmtConfigResult/IsOxfmtIgnoredOptions which are not exported by
load-oxfmt-config@0.4.1; either upgrade the dependency to a version that exports
those symbols, or change this file to use the 0.4.1 API (loadOxfmtConfig and
resolveOxfmtrcPath). If you choose the latter, remove the JSDoc type
IsOxfmtIgnoredOptions, replace the isOxfmtIgnored call with logic that loads the
config via loadOxfmtConfig/resolveOxfmtrcPath and derives ignore patterns (or
checks the ignorePath) using the pluginOptions (configPath, cwd,
disableNestedConfig, filepath, ignorePath, useCache, withNodeModules,
useConfig), and store results in the existing ignored variable so subsequent
code uses the same shape; otherwise bump the package version to one that exports
isOxfmtIgnored/loadOxfmtConfigResult and update imports accordingly.
| if (useConfig) { | ||
| const loaded = await loadOxfmtConfigResult({ | ||
| configPath: pluginOptions.configPath, | ||
| cwd: pluginOptions.disableNestedConfig ? cwd : dirname(filename), | ||
| editorconfig: pluginOptions.editorconfig, | ||
| useCache: pluginOptions.useCache, | ||
| }) | ||
| const { overrides: configOverrides, ...loadedConfig } = loaded.config | ||
| const { overrides: ruleOverrides, ...inlineOptionsWithoutOverrides } = | ||
| inlineFormatOptions | ||
| effectiveOverrides = [ | ||
| ...(configOverrides ?? []), | ||
| ...(Array.isArray(ruleOverrides) ? ruleOverrides : []), | ||
| ] | ||
| overrideBaseDir = loaded.dirname ?? overrideBaseDir | ||
|
|
||
| finalOptions = { | ||
| ...loadedConfig, | ||
| ...inlineOptionsWithoutOverrides, | ||
| } |
There was a problem hiding this comment.
Rule-level overrides merge contradicts the coding guideline for this file.
The useConfig branch now appends ruleOverrides after configOverrides (lines 158–161), but the repository coding guideline for workers/oxfmt.mjs states that, when useConfig is true, overrides come from loaded config and rule-level overrides are ignored by design. Either revert this branch to drop rule-level overrides under useConfig, or update the guideline so the two stay in sync — right now reviewers can't tell which is authoritative, and CLI-parity tests should pin the intended semantics.
♻️ Proposed fix to honor the current guideline
- const { overrides: configOverrides, ...loadedConfig } = loaded.config
- const { overrides: ruleOverrides, ...inlineOptionsWithoutOverrides } =
- inlineFormatOptions
- effectiveOverrides = [
- ...(configOverrides ?? []),
- ...(Array.isArray(ruleOverrides) ? ruleOverrides : []),
- ]
+ const { overrides: configOverrides, ...loadedConfig } = loaded.config
+ // Rule-level overrides are intentionally dropped when useConfig is true.
+ const { overrides: _ruleOverrides, ...inlineOptionsWithoutOverrides } =
+ inlineFormatOptions
+ effectiveOverrides = Array.isArray(configOverrides)
+ ? configOverrides
+ : undefinedAs per coding guidelines: "In workers/oxfmt.mjs, when useConfig is true, overrides come from loaded config; rule-level overrides are ignored by design."
🤖 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 `@workers/oxfmt.mjs` around lines 148 - 167, The code currently merges
rule-level overrides into effectiveOverrides when useConfig is true (see
loadOxfmtConfigResult, configOverrides, ruleOverrides, inlineFormatOptions, and
effectiveOverrides), which contradicts the guideline that rule-level overrides
must be ignored when useConfig is true; update the useConfig branch to set
effectiveOverrides strictly from the loaded config (e.g., effectiveOverrides =
configOverrides ?? []) and do not append or reference ruleOverrides there, while
keeping finalOptions built from loadedConfig and inline options as before.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 39 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- tests/fixtures/cli-parity/package-lock.json: Language not supported
- tests/fixtures/cli-parity/pnpm-lock.yaml: Language not supported
| const ruleIgnorePatterns = isStringArray(inlineFormatOptions.ignorePatterns) | ||
| ? inlineFormatOptions.ignorePatterns | ||
| : undefined | ||
|
|
||
| // Check if file matches the files patterns | ||
| const matches = getCachedMatcher(files)(relativePath) | ||
| if (ruleIgnorePatterns?.length && cwd) { |
|
|
||
| ## Behavior That Is Easy To Break | ||
|
|
||
| - In [workers/oxfmt.mjs](workers/oxfmt.mjs), when `useConfig` is true, `overrides` come from loaded config; rule-level `overrides` are ignored by design. |
ntnyq
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 39 changed files in this pull request and generated 4 comments.
Files not reviewed (2)
- tests/fixtures/cli-parity/package-lock.json: Language not supported
- tests/fixtures/cli-parity/pnpm-lock.yaml: Language not supported
| if (ignored.ignored) { | ||
| if ( | ||
| ruleIgnorePatterns?.length && | ||
| ignored.reason === 'config-ignore-patterns' | ||
| ) { |
|
|
||
| ## Behavior That Is Easy To Break | ||
|
|
||
| - In [workers/oxfmt.mjs](workers/oxfmt.mjs), when `useConfig` is true, `overrides` come from loaded config; rule-level `overrides` are ignored by design. |
| type FormatResult = Awaited<ReturnType<typeof format>> | ||
| type WorkerOptions = FormatConfig & LoadOxfmtConfigOptions | ||
|
|
||
| const runWorker = createSyncFn(join(dirWorkers, 'oxfmt.mjs')) as unknown as ( | ||
| filename: string, | ||
| sourceText: string, | ||
| options?: WorkerOptions, | ||
| options?: RuleOxfmtOptions, | ||
| ) => FormatResult |
| if (pluginOptions.respectOxfmtDefaultIgnores !== false && cwd) { | ||
| /** @type {import('load-oxfmt-config').IsOxfmtIgnoredOptions} */ | ||
| const ignoredOptions = { | ||
| configPath: pluginOptions.configPath, | ||
| cwd, | ||
| disableNestedConfig: pluginOptions.disableNestedConfig, | ||
| filepath: filename, | ||
| ignorePath: pluginOptions.ignorePath, | ||
| includeConfigIgnorePatterns: useConfig, | ||
| loadConfigForIgnorePatterns: useConfig, |
Summary by CodeRabbit
New Features
Documentation
Tests