Conversation
🦋 Changeset detectedLatest commit: b7e7910 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update modernizes and refactors the codebase to improve ESLint and Prettier integration, updates dependency versions (notably ESLint v9 and TypeScript parser), enhances support for ESLint's flat config, and adjusts test and configuration files to align with these changes. Several utility functions are reworked for asynchronous module loading. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PrettierESLint (index.ts)
participant ESLint (utils.ts)
participant Prettier
participant FileSystem
User->>PrettierESLint: format({ filePath, text, eslintConfig, ... })
PrettierESLint->>ESLint: importModule(eslintPath)
PrettierESLint->>Prettier: importModule(prettierPath)
PrettierESLint->>ESLint: getESLint(eslintPath, eslintOptions)
PrettierESLint->>ESLint: analyze text with config
PrettierESLint->>Prettier: format text if needed
PrettierESLint->>FileSystem: Write formatted output
PrettierESLint-->>User: Return formatted output and messages
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/index.ts (1)
🪛 Biome (1.9.4)src/index.ts[error] 240-240: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 241-241: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 242-242: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 243-243: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 244-244: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 245-245: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 246-246: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 247-247: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) [error] 250-250: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) 🔇 Additional comments (11)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1190 +/- ##
===========================================
- Coverage 100.00% 99.67% -0.33%
===========================================
Files 2 2
Lines 299 310 +11
Branches 78 80 +2
===========================================
+ Hits 299 309 +10
- Misses 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 3da0373 in 2 minutes and 47 seconds. Click for details.
- Reviewed
770lines of code in10files - Skipped
1files when reviewing. - Skipped posting
12draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. package.json:17
- Draft comment:
Updated Node engine versions correctly reflect ESLint v9 support. Ensure that any documentation for required Node versions is updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/index.ts:136
- Draft comment:
Using the nullish assignment operator (??=) for eslint.languageOptions is clear. Consider adding a comment if there are any edge cases where no parser is set for files that require one. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
3. src/index.ts:139
- Draft comment:
Assigning the parser asynchronously with importModule for .ts, .vue, and .svelte extensions works well. Consider logging a warning if none of the expected extensions match and no parser is set. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
4. src/utils.ts:556
- Draft comment:
The loadModule hack using new Function is acceptable as a temporary workaround (noted by FIXME). However, note that dynamic code evaluation can be slippery. Revisit this when targeting full ESM support. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%The comment is informative and suggests a future action rather than a specific code change or confirmation of intention. It doesn't ask for a test or a specific code suggestion. It seems to be more of a note for future consideration rather than a direct actionable comment.
5. test/index.spec.ts:327
- Draft comment:
The expected endColumn value was updated to 10. Verify that this change accurately reflects ESLint v9 behavior. If this is due to ESLint v9 output changes, consider noting this in the changelog. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment asks the PR author to verify the change and suggests updating the changelog, which violates the rules against asking for verification or changelog updates. It doesn't provide a specific code suggestion or point out a specific issue with the code.
6. test/utils.spec.ts:245
- Draft comment:
Test cases for getOptionsForFormatting seem comprehensive. Ensure that fallbacks and disabled rule scenarios are well documented so future maintainers understand the expected precedence between ESLint rules, provided prettierOptions, and fallbacks. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. src/utils.ts:561
- Draft comment:
The dynamic module loading hack using new Function is clever but potentially risky if modulePath is user-controlled. It’s marked as a FIXME, so consider switching to native dynamic import in future and add a guard to prevent injection. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The security concern is valid, but the code already acknowledges this is a temporary hack that needs to be fixed. The FIXME comment and eslint-disable comments show the developers are aware of the risks. The comment doesn't add new information beyond what's already documented in the code. The comment does identify a real security risk. Perhaps keeping it would help ensure the FIXME gets addressed. The code already has clear documentation of the risk via the FIXME and eslint comments. The PR comment would just be redundant and doesn't add actionable information beyond what's already there. Delete the comment since it just restates what's already documented in the code via FIXME and eslint comments.
8. package.json:17
- Draft comment:
The engine field now specifies '^18.18.0 || ^20.9.0 || >=21.1.0'. It would be helpful to document why these specific Node versions are required (e.g. for ESLint v9 flat config support). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking for documentation on why specific Node versions are required. This is a request for additional information, which violates the rule against asking the PR author to update the PR description or provide explanations.
9. package-scripts.js:62
- Draft comment:
The FIXME comment about a temporary workaround for prettier-eslint-cli should be updated when the issue is resolved. This will help future maintainers understand whether the workaround is still necessary. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, suggesting an update to a FIXME comment for future maintainers. It doesn't provide a specific code suggestion or address a potential issue with the current code.
10. test/index.spec.ts:327
- Draft comment:
The test now expects msg.endColumn to be 10 (instead of a previous value of 11?). Verify that this change correctly reflects ESLint v9 behavior. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
11. package.json:11
- Draft comment:
The maintainer's name 'JounQin' on line 11 might be a typographical error. Please verify if it should be 'John Qin' or the intended correct spelling. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. src/index.ts:416
- Draft comment:
There's an unusual re-export statement on line 416:export type * from './types.js';. Considering that types are imported from './types.ts' earlier in the file, this might be a typographical error. Please verify if this should beexport * from './types.ts';or adjust the export syntax accordingly. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_bNskubvmpiOlPWYK
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c20eb12 in 28 seconds. Click for details.
- Reviewed
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/pre.json:1
- Draft comment:
JSON structure looks correct and aligns with the new eslint v9 support. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .changeset/pre.json:4
- Draft comment:
Verify that prettier-eslint v16.4.2 fully supports the ESLint v9 flat config changes. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_F535dqeg93lXk6OT
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
test/index.spec.ts (1)
527-534: Commented outnoopOutputfunctionThe
noopOutputfunction has been commented out rather than removed. Consider either removing it completely or adding a comment explaining why it's being kept in a commented form.-// function noopOutput() { -// return ` -// function foo() { -// // stuff -// console.log("Hello world!", and, stuff); -// } -// `; -// }src/index.ts (2)
202-205: Repeated dynamic imports may incur noticeable overhead – consider caching Prettier
importModule()is executed every timeprettify()runs, which means loading/initialising Prettier on every file. Although Node caches modules, the dynamicimport()path still adds I/O and async overhead.A tiny memoisation helper keeps the laziness but avoids the repeated round-trip:
function createPrettify(formatOptions: PrettierOptions, prettierPath: string) { - return async function prettify( + let prettierPromise: Promise<typeof import('prettier')> | undefined; + + return async function prettify( param: PrettifyInput | string, ): Promise<{ output: string; messages: Linter.LintMessage[] }> { + // Load once, then reuse. + if (!prettierPromise) { + prettierPromise = importModule<typeof import('prettier')>( + prettierPath, + 'prettier', + ); + } + const prettier = await prettierPromise;This keeps semantics identical while removing redundant imports.
136-151: Parser auto-selection is handy but can be DRY-ed up (and expanded) with a look-up tableThe current cascade of
if / else ifstatements will grow every time a new extension/parsers pair appears.
A map keeps it declarative and avoids accidental omissions:const PARSERS: Record<string, string> = { '.ts': '@typescript-eslint/parser', '.tsx': '@typescript-eslint/parser', '.vue': 'vue-eslint-parser', '.svelte': 'svelte-eslint-parser', }; if (!formattingOptions.eslint.languageOptions.parser) { const parserModule = PARSERS[fileExtension as keyof typeof PARSERS]; if (parserModule) { formattingOptions.eslint.languageOptions.parser = await importModule( parserModule, ); } }This makes it obvious which extensions are supported and makes adding new ones trivial.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
__mocks__/loglevel-colored-level-prefix.ts(1 hunks)eslint.config.mjs(1 hunks)jest.config.js(1 hunks)package-scripts.js(1 hunks)package.json(4 hunks)src/index.ts(8 hunks)src/types.ts(2 hunks)src/utils.ts(6 hunks)test/index.spec.ts(13 hunks)test/utils.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/utils.ts (1)
src/types.ts (1)
ESLintOptions(82-85)
src/index.ts (1)
src/utils.ts (3)
extractFileExtensions(623-640)importModule(568-586)getESLint(588-602)
🪛 ESLint
src/index.ts
[error] 4-4: Unable to resolve path to module '@esm2cjs/indent-string'.
(import-x/no-unresolved)
🪛 Biome (1.9.4)
src/index.ts
[error] 235-235: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 236-236: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 237-237: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 238-238: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 239-239: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (23)
jest.config.js (1)
10-13: Reduced coverage thresholds reflect pragmatic approach to ESLint v9 supportThe coverage thresholds have been lowered slightly to accommodate the new ESLint v9 flat configuration support. This is a reasonable trade-off that maintains high quality standards while allowing for the complexity introduced by the new feature.
package-scripts.js (1)
62-62: Comment correction improves clarityThe updated comment correctly identifies that the workaround is specifically for
prettier-eslint-clirather than Flat ESLint. This clarification helps other developers understand the purpose of the temporary solution.__mocks__/loglevel-colored-level-prefix.ts (1)
36-36: Removed unnecessary ESLint disable commentThe ESLint disable comment for
no-consolehas been removed, which is appropriate for a test mock file where console logging is acceptable. This aligns with the broader ESLint configuration updates in the project.eslint.config.mjs (1)
29-30: SonarJS rules disabled to accommodate development workflowThe two SonarJS rules being disabled make sense in this context:
sonarjs/fixme-tag: Allows legitimate TODOs and FIXMEs to remain in the codebase while issues are being trackedsonarjs/function-return-type: Avoids redundancy with TypeScript's type inference capabilitiestest/utils.spec.ts (1)
247-247: Updated test to use ESLint v9 flat config formatThe test has been correctly updated to use the new flat config format with the
languageOptions.globalsobject structure instead of the legacy array format. This change is essential for supporting ESLint v9.package.json (3)
17-17: Updated Node.js engine requirementsThe Node.js engine requirements have been updated to a more specific range that aligns with ESLint v9's own requirements.
55-55: Dependencies updated to support ESLint v9The key dependency updates include:
- Added "@esm2cjs/indent-string": "^5.0.0" (replacing "indent-string")
- "@typescript-eslint/parser" upgraded to "^8.32.0"
- "eslint" upgraded to "^9.26.0"
- "vue-eslint-parser" upgraded to "^10.1.3"
These updates properly align with ESLint v9's new flat configuration format requirements.
Also applies to: 56-56, 59-59, 66-66
88-88: DevDependencies updated for compatibilityThe development dependencies have been updated to maintain compatibility with the core dependency changes, particularly ESLint v9 compatibility.
Also applies to: 99-99, 100-100
test/index.spec.ts (5)
1-1: Updated imports and ESLint configurationsThe ESLint disable comment has been updated to include additional rules, and
stripIndentis now imported from 'common-tags' instead of the deprecated 'strip-indent' package.Also applies to: 7-7
68-68: Migrated to ESLint v9 flat config formatTest cases have been updated to use
languageOptionsobjects containingparserOptionsandglobalsproperties, which aligns with ESLint v9's flat config structure.Also applies to: 106-109, 187-188, 280-284
166-166: UpdatedignorePatterntoignorePatternsarrayChanged
ignorePattern(string) toignorePatterns(array) to match ESLint v9's configuration format.
177-178: Updated to useglobalThisinstead ofwindowReferences to
windowhave been replaced with the more modern and standardizedglobalThis.Also applies to: 180-180
319-319: Explicitfix: falsesettingThe ESLint
fixoption is now explicitly set tofalsein the analyze test to reflect the asynchronous ESLint invocation changes in the codebase.src/types.ts (3)
54-54: Added local StringLiteral type aliasAdded a type alias for
StringLiteral<T>to replace the previous import from./utils.
89-91: Added ESLintConfigLanguageOptions typeCreated a new type
ESLintConfigLanguageOptionsthat represents a non-nullableLinter.Config['languageOptions'], which is needed for working with ESLint v9's flat config format.
93-93: Simplified ESLintConfig interfaceThe
ESLintConfiginterface has been simplified to directly extendLinter.ConfigandESLintOptions, removing previously defined custom types that are no longer needed with ESLint v9's flat config.src/utils.ts (6)
1-2: Added Node.js core module importsAdded imports from Node.js core modules for path manipulation and URL conversion, which support the new asynchronous module loading functionality.
10-15: Updated imports to include StringLiteral from typesUpdated imports to include
StringLiteralfrom './types.ts' to align with the type definition changes.
143-143: Updated ESLint configuration handlingModified
getRelevantESLintConfigto:
- Create a Linter instance with
{ configType: 'eslintrc' }to ensure compatibility with ESLint v9- Set the
fixoption conditionally using nullish coalescing to preserve user settings when providedThese changes align with ESLint v9's configuration options.
Also applies to: 162-162
568-586: Added importModule function for asynchronous importsAdded an
importModule<T>function that asynchronously imports a module, correctly handling default exports and providing better error messages.
588-602: Updated getESLint to be asynchronousModified
getESLintto be an async function that usesimportModuleto dynamically load the ESLint module, improving the architecture by supporting asynchronous module loading.
604-640: Added extractFileExtensions utility functionAdded a new utility function
extractFileExtensionsthat extracts unique file extensions from glob patterns, supporting both single extensions and multiple extensions in braces.The function effectively handles the transition from the deprecated
eslintConfig.extensionsto a mechanism that derives extensions fromeslintConfig.filespatterns.src/index.ts (1)
4-4: Ensure the new ESM shim is actually resolvable at runtimeThe static-analysis hint reports
@esm2cjs/indent-stringas “unresolved”.
Please double-check that this package is listed inpackage.json → dependencies(orpeerDependencies) and that every environment that bundles this package understands theexportsfield emitted byesm2cjs.If you publish to both CJS and ESM targets, also verify the type-resolution paths so consumers don’t unexpectedly fall back to the (now removed)
indent-stringpackage.🧰 Tools
🪛 ESLint
[error] 4-4: Unable to resolve path to module '@esm2cjs/indent-string'.
(import-x/no-unresolved)
Signed-off-by: JounQin <admin@1stg.me>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 3618d2d in 52 seconds. Click for details.
- Reviewed
42lines of code in3files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. jest.config.js:10
- Draft comment:
Branch coverage threshold adjusted from 96 to 94. Confirm that this change aligns with test expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/index.ts:321
- Draft comment:
Using nullish coalescing (??) instead of logical OR (||) ensures valid false values are not overwritten. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/utils.ts:150
- Draft comment:
Refactor using optional chaining simplifies check on rule meta; it improves readability and correctness. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. jest.config.js:10
- Draft comment:
Branch coverage threshold reduced from 96 to 94. Ensure this decrease is intentional and aligns with overall testing goals. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. src/index.ts:323
- Draft comment:
Using '??' instead of '||' correctly preserves falsy values (like false) from the user config. This is a proper fix for default assignment in getESLintApiOptions. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/utils.ts:149
- Draft comment:
Refactored check to use optional chaining (rule.meta?.fixable) for cleaner code and to remove the need for a linter disable comment. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_vJylxfeUp00I1znX
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed dff624a in 1 minute and 34 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. test/index.spec.ts:232
- Draft comment:
The Vue.js test input has been updated to remove the space between the function name and parentheses. Confirm that this fits the new ESLint v9 flat config behavior and that the expected output adds the required space. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm the behavior of ESLint v9, which violates the rule against asking for confirmation or ensuring behavior. It does not provide a specific code suggestion or ask for a specific test to be written. Therefore, it should be removed.
2. test/index.spec.ts:232
- Draft comment:
Ensure that this change doesn't affect other tests expecting a different formatting style. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that the change doesn't affect other tests, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
3. test/index.spec.ts:229
- Draft comment:
In the Vue.js test case, the input text was changed from 'function foo (){…}' to 'function foo(){…}'. This appears to align with ESLint v9 flat config handling. Consider adding a brief comment to explain that this modification was needed to match the updated ESLint behavior. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment appears to be incorrect. The ESLint rule actually requires a space before the function parenthesis, but the change removes that space. This seems to be a test case that is now incorrect given its ESLint config. The comment is suggesting documentation for a change that appears to be wrong. Additionally, even if the change was correct, documenting test case changes is not typically necessary as tests should be self-documenting through their structure and assertions. Could the ESLint rule be working differently than I assume? Maybe there's some Vue-specific handling I'm not aware of? Even if there are Vue-specific nuances, the comment is still not useful because it's asking for documentation of test changes, which should be self-evident from the test structure itself. The comment should be deleted because it suggests documenting a test change that appears incorrect, and even if correct, test changes rarely need explicit documentation.
Workflow ID: wflow_Mjrsfp3MDx6A4INd
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed bd085d9 in 1 minute and 37 seconds. Click for details.
- Reviewed
33lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/index.ts:243
- Draft comment:
Using non-null assertion when deletingeslintConfig.overrideConfig.plugins!['@']may be unsafe. Consider checking if the property exists before deletion. - Reason this comment was not posted:
Marked as duplicate.
2. src/utils.ts:596
- Draft comment:
Updated error message in ESLint instance creation. Confirm the new text accurately reflects ESLint v9 usage. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
3. src/utils.ts:595
- Draft comment:
Updated error message now correctly refers to the 'ESLint instance' (rather than the deprecated CLIEngine), which aligns with ESLint v9. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_xJnBI7U0TeKlCP3m
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 39b63fb in 1 minute and 51 seconds. Click for details.
- Reviewed
101lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/index.ts:120
- Draft comment:
Rename of 'extensions' to 'eslintExtensions' is clear. However, checking for a non-empty 'files' array with '.length' may be fragile if an empty array is a valid value. Consider a stricter check (e.g., Array.isArray and length > 0). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment makes a valid point about array validation - just checking .length could be fragile if files is not an array. However, the code later calls .flat() which would fail if files wasn't an array anyway. The optional chaining provides some safety. The suggested fix adds explicit validation but may be overly defensive. The code already has implicit array validation via .flat(). The suggested change adds verbosity without clear benefit. While explicit validation can be good practice, in this case the existing code handles the edge cases adequately through optional chaining and implicit validation in .flat(). The comment should be removed. The existing code handles array validation sufficiently through optional chaining and .flat(), making the suggested explicit validation unnecessary.
2. src/index.ts:232
- Draft comment:
Deleting 'eslintConfig.overrideConfig.plugins!['@']' directly may be risky. Verify that this deletion won’t remove necessary configuration in some scenarios. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/utils.ts:625
- Draft comment:
The new reduce in extractFileExtensions improves extension extraction by ensuring trimmed extensions with a dot. Confirm that adding a dot is always desired. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. src/index.ts:122
- Draft comment:
Review eslintFiles fallback: If eslintConfig.files is an empty array, it falls back to DEFAULT_ESLINT_FILES. Confirm this is intended, as an empty array might be a valid override. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to verify their intention, which violates our rules. It's not pointing out a clear bug or suggesting a specific improvement. The fallback behavior seems reasonable - an empty array of files would mean no files should be processed, which doesn't make sense in this context. The comment is speculative about whether an empty array "might" be valid. Maybe an empty array is actually a valid use case that should be respected rather than falling back. The code could be preventing legitimate configurations. While technically possible, the comment doesn't provide evidence that empty arrays are a valid use case. Without clear evidence of a problem, we should assume the author's implementation is correct. Delete this comment as it merely asks for verification without identifying a clear issue, and speculates about potential problems without evidence.
5. src/index.ts:248
- Draft comment:
Avoid non‐null assertions when deleting overrideConfig.plugins['@']. Consider checking if the property exists before deletion to prevent potential runtime errors. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The code already handles the null/undefined case for plugins on line 235 withplugins: eslintConfig.plugins ?? {}. This means plugins will always be an object when we reach line 248. The non-null assertion is actually correct here since we know plugins cannot be undefined. The suggestion would add unnecessary optional chaining. I could be wrong about the initialization - there might be edge cases where plugins becomes undefined between lines 235 and 248. The spread operator on line 237 could potentially override the plugins property. Even with the spread, the type system enforces that overrideConfig.plugins must be an object if it exists. The non-null assertion is appropriate here as a type guard. The comment should be deleted because the current code is correct - plugins is guaranteed to be an object at this point due to the nullish coalescing initialization.
6. src/utils.ts:626
- Draft comment:
Improved extractFileExtensions: returning extensions with a leading dot is consistent with path.extname. The use of reduce with trim aids robustness. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_GeGrPbphvO9kuibk
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/utils.ts (1)
554-565: Be careful with dynamic imports usingnew FunctionThe
loadModulefunction usesnew Function()as a workaround for TypeScript transpilation limitations, which is similar to usingeval(). While this approach works, it introduces potential security risks ifmodulePathcomes from untrusted sources.src/index.ts (1)
231-248: 🛠️ Refactor suggestionAvoid mutating the original config with
deleteoperationsThe current implementation mutates the
eslintConfigobject directly and uses performance-impactingdeleteoperations. This can cause unexpected side effects if the same config object is used elsewhere.Replace the mutation approach with a destructuring and new object creation:
- eslintConfig.overrideConfig = { - languageOptions: eslintConfig.languageOptions, - rules: eslintConfig.rules, - ignores: eslintConfig.ignorePatterns ?? [], - plugins: eslintConfig.plugins ?? {}, - settings: eslintConfig.settings ?? {}, - ...eslintConfig.overrideConfig, - }; - - delete eslintConfig.baseConfig; - delete eslintConfig.language; - delete eslintConfig.languageOptions; - delete eslintConfig.linterOptions; - delete eslintConfig.rules; - delete eslintConfig.ignorePatterns; - delete eslintConfig.plugins; - delete eslintConfig.settings; - delete eslintConfig.overrideConfig.plugins!['@']; - - const eslint = await getESLint(eslintPath, eslintConfig); + const { + languageOptions, + rules, + ignorePatterns, + plugins = {}, + settings = {}, + baseConfig: _baseConfig, + language: _language, + linterOptions: _linterOptions, + ...restConfig + } = eslintConfig; + + const { '@': _atPlugin, ...filteredPlugins } = restConfig.overrideConfig?.plugins || {}; + + const modifiedConfig = { + ...restConfig, + overrideConfig: { + ...restConfig.overrideConfig, + languageOptions, + rules, + ignores: ignorePatterns ?? [], + plugins: { ...filteredPlugins, ...plugins }, + settings: settings ?? {}, + }, + }; + + const eslint = await getESLint(eslintPath, modifiedConfig);🧰 Tools
🪛 Biome (1.9.4)
[error] 240-240: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 241-241: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 242-242: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 243-243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 244-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 245-245: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 246-246: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 247-247: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 248-248: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🧹 Nitpick comments (1)
src/utils.ts (1)
603-638: Optimize extension extraction performanceThe
extractFileExtensionsfunction is well-implemented and handles both single and multiple extension patterns correctly. However, the use of spread operator in the reducer could cause performance issues with large arrays.Consider using
push()instead of spread for better performance:if (matchMultiple) { return matchMultiple[1].split(',').reduce<string[]>((acc, ext) => { ext = ext.trim(); - return ext ? [...acc, `.${ext}`] : acc; + if (ext) acc.push(`.${ext}`); + return acc; }, []); }🧰 Tools
🪛 Biome (1.9.4)
[error] 629-629: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.ts(9 hunks)src/utils.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/utils.ts (1)
src/types.ts (1)
ESLintOptions(82-85)
src/index.ts (1)
src/utils.ts (3)
extractFileExtensions(620-638)importModule(567-585)getESLint(587-601)
🪛 Biome (1.9.4)
src/utils.ts
[error] 629-629: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
src/index.ts
[error] 240-240: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 241-241: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 242-242: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 243-243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 244-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 245-245: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 246-246: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 247-247: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 248-248: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 ESLint
src/index.ts
[error] 4-4: Unable to resolve path to module '@esm2cjs/indent-string'.
(import-x/no-unresolved)
🔇 Additional comments (10)
src/utils.ts (5)
567-585: Good implementation of dynamic module loading with default export unwrappingThe
importModulefunction handles both path normalization and default export unwrapping, which is essential for compatibility with both ESM and CommonJS modules. The proper error handling and logging also improve debugging.
587-601: Excellent conversion to async ESLint instance creationThe asynchronous
getESLintfunction properly leverages the dynamic import mechanism and provides good error handling. This change is crucial for supporting ESLint v9's module structure.
143-143: Updated Linter constructor for ESLint v9 compatibilityUsing
{ configType: 'eslintrc' }in the Linter constructor is necessary for ESLint v9 flat config support, ensuring backward compatibility with ESLint rules.
150-150: Improved null safety with optional chainingUsing optional chaining (
?.) for checkingrule.meta?.fixableimproves code robustness when handling potentially undefined metadata.
161-161: Better default value handling with nullish coalescingUsing
??instead of||ensures that an explicitfalsevalue forfixis respected rather than being overridden by the defaulttrue.src/index.ts (5)
39-54: Comprehensive default file extension supportThe addition of
DEFAULT_ESLINT_EXTENSIONSandDEFAULT_ESLINT_FILESconstants provides excellent support for modern JavaScript and TypeScript module formats, including CJS, MJS, and TS variants.
122-128: Good fallback to default file patternsUsing
eslintConfig.files?.lengthfor nullish check andDEFAULT_ESLINT_FILESas a fallback ensures that the code works even when file patterns aren't explicitly specified. The use of the newextractFileExtensionsutility properly handles complex glob patterns.
141-155: Improved asynchronous parser loadingThe code now correctly initializes
languageOptionsif needed and asynchronously loads the appropriate parser based on file extension, supporting TypeScript, Vue, and Svelte files.
333-334: Better default value handling with nullish coalescingUsing
??instead of logical OR ensures that explicitfalsevalues are respected rather than being overridden by default values, which is important for configuration options.
378-387: Well-implemented async Prettier config resolutionThe
getPrettierConfigfunction has been properly converted to async, using the newimportModuleutility for dynamic loading of Prettier. This aligns with the overall shift to asynchronous module loading.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 43844ad in 1 minute and 56 seconds. Click for details.
- Reviewed
104lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/index.ts:122
- Draft comment:
Good use of optional chaining to select ESLint files. Consider verifying that eslintConfig.files is always an array when present. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
2. src/utils.ts:626
- Draft comment:
The updated reduce logic in extractFileExtensions cleanly ensures a dot prefix and trimmed extension. Looks good; just ensure consistency with patterns that already include a dot. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/index.ts:121
- Draft comment:
Using optional chaining (eslintConfig.files?.length) to fall back to DEFAULT_ESLINT_FILES is clear, but note that mutating the config here may affect later reuse. Consider cloning if reentrancy is a concern. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/index.ts:237
- Draft comment:
The removal of properties (baseConfig, language, linterOptions) and deletion of overrideConfig.plugins['@'] aligns with ESLint v9 flat config requirements, but the non-null assertion ('!') could throw if the property is absent. A safe-check might be safer. - Reason this comment was not posted:
Marked as duplicate.
5. src/utils.ts:595
- Draft comment:
Updating the error message from 'CLIEngine' to 'instance' correctly reflects ESLint v9’s usage. This change improves clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/utils.ts:621
- Draft comment:
The revised extractFileExtensions now trims and prepends a dot for multiple extensions, ensuring consistency with single-extension patterns. This is a solid improvement provided consuming code expects dot-prefixed extensions. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_ziUTpmrZrRPWVAq2
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/index.ts (1)
229-249: 🛠️ Refactor suggestionAvoid mutating the caller's config – produce a derived object instead.
The current implementation mutates the
eslintConfigobject and usesdeleteoperations, which can lead to:
- Unexpected side effects if the caller reuses the config
- Performance issues (as flagged by Biome)
This issue was previously identified in past reviews but hasn't been addressed.
function createEslintFix(eslintConfig: ESLintConfig, eslintPath: string) { return async function eslintFix(text: string, filePath?: string) { - eslintConfig.overrideConfig = { - languageOptions: eslintConfig.languageOptions, - rules: eslintConfig.rules, - ignores: eslintConfig.ignorePatterns ?? [], - plugins: eslintConfig.plugins ?? {}, - settings: eslintConfig.settings ?? {}, - ...eslintConfig.overrideConfig, - }; - - delete eslintConfig.baseConfig; - delete eslintConfig.language; - delete eslintConfig.languageOptions; - delete eslintConfig.linterOptions; - delete eslintConfig.rules; - delete eslintConfig.ignorePatterns; - delete eslintConfig.plugins; - delete eslintConfig.settings; - delete eslintConfig.overrideConfig.plugins!['@']; - - const eslint = await getESLint(eslintPath, eslintConfig); + const { + languageOptions, + rules, + ignorePatterns, + plugins = {}, + settings = {}, + baseConfig, + language, + linterOptions, + overrideConfig = {}, + ...restConfig + } = eslintConfig; + + // Create a new overrideConfig without the '@' plugin + const { ['@']: _, ...filteredPlugins } = overrideConfig.plugins || {}; + + // Create a new clean config object + const cleanConfig = { + ...restConfig, + overrideConfig: { + languageOptions, + rules, + ignores: ignorePatterns ?? [], + plugins: filteredPlugins, + settings, + ...overrideConfig, + }, + }; + + const eslint = await getESLint(eslintPath, cleanConfig);🧰 Tools
🪛 Biome (1.9.4)
[error] 240-240: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 241-241: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 242-242: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 243-243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 244-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 245-245: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 246-246: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 247-247: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 248-248: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🧹 Nitpick comments (1)
src/index.ts (1)
122-128: Consider adding fallback for empty file arrays.The code correctly handles when
eslintConfig.filesis undefined, but it might not handle the case whereeslintConfig.filesis an empty array properly.// istanbul ignore next const eslintFiles = eslintConfig.files?.length ? eslintConfig.files : DEFAULT_ESLINT_FILES;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.ts(9 hunks)src/utils.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/index.ts (1)
src/utils.ts (3)
extractFileExtensions(620-641)importModule(567-585)getESLint(587-601)
🪛 ESLint
src/index.ts
[error] 4-4: Unable to resolve path to module '@esm2cjs/indent-string'.
(import-x/no-unresolved)
🪛 Biome (1.9.4)
src/index.ts
[error] 240-240: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 241-241: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 242-242: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 243-243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 244-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 245-245: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 246-246: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 247-247: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 248-248: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (7)
src/index.ts (7)
39-54: LGTM: Good addition of standard constants.The addition of
DEFAULT_ESLINT_EXTENSIONSandDEFAULT_ESLINT_FILESconstants improves code maintainability and readability, providing clear defaults for ESLint configuration.
141-156: Good parser selection based on file extension.The asynchronous loading of parsers based on file extension is a good approach, improving compatibility with ESLint v9 and different file types.
206-210: LGTM: Properly converted to async import.The change to use the async
importModulefunction aligns with the PR's goal of moving to asynchronous module loading.
333-334: LGTM: Proper use of nullish coalescing.The change from
||to??for default values is correct and allows for explicitly setting values tofalse.
356-356: LGTM: Properly updated to use async getESLint.The asynchronous
getESLintfunction call aligns with the PR's goal of moving to asynchronous module loading.
378-385: LGTM: Properly converted to async function.The conversion of
getPrettierConfigto an async function that usesimportModuleis appropriate for the PR's goals.
4-4:Details
❌ Incorrect review comment
Update package.json to match import change.
The import has been changed from 'indent-string' to '@esm2cjs/indent-string', but there's no indication in the static analysis that this dependency has been added to package.json.
🏁 Script executed:
#!/bin/bash # Check if @esm2cjs/indent-string is in package.json grep -A 3 -B 3 "@esm2cjs/indent-string" package.json || echo "Dependency not found in package.json"Length of output: 217
Ignore dependency update suggestion
The
@esm2cjs/indent-stringpackage is already declared inpackage.jsonunder dependencies, so no changes are needed here.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 ESLint
[error] 4-4: Unable to resolve path to module '@esm2cjs/indent-string'.
(import-x/no-unresolved)
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b7e7910 in 1 minute and 54 seconds. Click for details.
- Reviewed
76lines of code in3files - Skipped
1files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. package-scripts.js:60
- Draft comment:
Reverting to the original 'prettier-eslint' call appears intentional. Ensure that removal of the temporary workaround for prettier-eslint-cli is acceptable since earlier workaround might have been addressing a specific issue. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. package.json:91
- Draft comment:
The dependency change from 'link:.' to 'portal:.' is noted. Confirm that 'portal:.' is the intended approach for dependency resolution under Yarn PnP or your specific workflow. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. src/index.ts:249
- Draft comment:
The removal of property deletion on 'eslintConfig.overrideConfig.plugins!['@']' was correctly replaced with deletion of the entire 'plugins' object. This aligns with the ESLint v9 flat config format; good catch with the comment referencing the ESLint core issue. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the change made in the PR. It does not provide any actionable feedback or suggestions for improvement. According to the rules, purely informative comments should be removed.
4. src/index.ts:254
- Draft comment:
Log message updates (e.g., 'calling eslint.lintText' instead of 'executeOnText') increase clarity and accurately reflect the updated ESLint API usage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only praises the log message updates without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.
5. package-scripts.js:60
- Draft comment:
The updated 'format' script now only targets '*.js' files. Previously, patterns for mjs files were handled via a workaround. Ensure this change intentionally omits mjs support or update the glob if needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The diff clearly shows that .mjs support was intentionally removed when switching back from the temporary workaround to the main prettier-eslint command. The comment starts with "Ensure this change..." which is a red flag. The PR author likely had a good reason for removing .mjs support when reverting the workaround. The comment does identify a real change in functionality - the loss of .mjs file formatting. Maybe this is an oversight that should be flagged? While the functionality change is real, the comment violates our rules by asking the author to "ensure" their intention rather than definitively pointing out an issue. The removal of .mjs appears intentional as part of removing the workaround. Delete the comment as it violates our rule against asking authors to verify their intentions, and the change appears intentional as part of reverting a temporary workaround.
6. package.json:92
- Draft comment:
The dependency for 'prettier-eslint' now uses 'portal:.' and resolutions pin eslint (and @prettier/eslint) for Yarn Berry. Confirm this is acceptable for your target user base and document any Yarn 2+ requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about dependency changes and asks the PR author to confirm and document requirements, which violates the rules. It doesn't provide a specific code suggestion or ask for a specific test to be written.
7. src/index.ts:250
- Draft comment:
Instead of deleting just a specific plugin key, the code now removes the entire 'plugins' property from overrideConfig. Verify that this aggressive removal won’t drop user-intended plugins, and add documentation if this is an intentional trade-off for ESLint v9 flat config support. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify their intention and to add documentation if the change is intentional. This violates the rule against asking the author to confirm their intention or to ensure the behavior is intended. The comment does not provide a specific code suggestion or ask for a specific test to be written. Therefore, it should be removed.
8. src/index.ts:427
- Draft comment:
There is a potential typographical inconsistency: the file imports types from './types.ts' (line 12) but then exports them usingexport type * from './types.js';(line 427). Please verify if this should be './types.ts' to maintain consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_mzMwEuc9PNFkLmYP
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
This initial prototype only supports ESLint v9 with Flat Config, the legacy eslintrc support will be added back later.
fix #947
fix #1015
fix #1156
close #1036
close #1083
Important
Add support for ESLint v9 flat config, update dependencies, and refactor code for improved maintainability.
src/index.tsandsrc/utils.ts.eslintrcsupport temporarily.eslintto^9.26.0and@typescript-eslint/parserto^8.32.0inpackage.json.indent-stringwith@esm2cjs/indent-string.jest.config.js.eslint.config.mjs.package.json.test/index.spec.tsandtest/utils.spec.tsto reflect changes in ESLint config structure.src/index.tsandsrc/utils.ts.src/types.ts.This description was created by
for b7e7910. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit