Skip to content

fix: use overrideConfigFile: true + overrideConfig instead#1193

Merged
JounQin merged 2 commits intomasterfrom
fix/overrideConfig
May 16, 2025
Merged

fix: use overrideConfigFile: true + overrideConfig instead#1193
JounQin merged 2 commits intomasterfrom
fix/overrideConfig

Conversation

@JounQin
Copy link
Copy Markdown
Member

@JounQin JounQin commented May 16, 2025

related eslint/eslint#19722


Important

Set overrideConfigFile to true and adjust overrideConfig in createEslintFix to address ESLint issue #19722.

  • Behavior:
    • In createEslintFix in index.ts, set eslintConfig.overrideConfigFile to true.
    • Modify eslintConfig.overrideConfig to include languageOptions, rules, ignores, plugins, and settings.
    • Delete eslintConfig.overrideConfig.plugins!['@'] to address ESLint issue #19722.
  • Tests:
    • Update index.spec.ts to reflect changes in ESLint configuration handling.
    • Change expected output in several test cases to match new behavior.
    • Remove sonarjs/no-commented-code disable comment in index.spec.ts.

This description was created by Ellipsis for 520166c. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of ESLint configuration overrides to ensure proper application and compatibility with specific plugin keys.
  • Tests

    • Updated test cases to reflect changes in expected formatting results.
    • Activated and refined a helper function for generating no-operation output in tests.

@JounQin JounQin requested a review from Copilot May 16, 2025 10:20
@JounQin JounQin self-assigned this May 16, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 16, 2025

🦋 Changeset detected

Latest commit: f7c0ded

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
prettier-eslint Patch

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented May 16, 2025

Walkthrough

A new changeset file was added to document a patch-level fix. In the main source, the ESLint configuration override logic was updated to use overrideConfigFile: true and to more specifically remove a problematic plugin key. Test cases were adjusted to expect different outputs, and a previously commented-out helper function was activated.

Changes

File(s) Change Summary
.changeset/late-eels-march.md Added a changeset file describing a patch-level fix related to configuration overrides.
src/index.ts Updated createEslintFix to set overrideConfigFile: true, refined plugin key deletion, and updated related comments.
test/index.spec.ts Simplified ESLint disable comment, updated expected outputs in several tests, and activated the noopOutput() function.

Poem

In the warren, code hops anew,
Configs override, as rabbits do.
Plugins pruned with careful paw,
Tests now pass—oh, what I saw!
With every patch, our garden grows,
The neatest code a bunny knows.
🐇✨

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

yarn install v1.22.22
[1/4] Resolving packages...
error Couldn't find any versions for "prettier-eslint" that matches "portal:."
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 5c03479 and f7c0ded.

📒 Files selected for processing (3)
  • .changeset/late-eels-march.md (1 hunks)
  • src/index.ts (2 hunks)
  • test/index.spec.ts (5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/index.ts

[error] 252-252: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (8)
.changeset/late-eels-march.md (1)

1-6: LGTM! The changeset description is clear and concise.

The changeset file correctly documents this as a patch-level fix, clearly stating the change to use overrideConfigFile: true + overrideConfig instead of the previous approach.

src/index.ts (2)

231-232: LGTM! Setting overrideConfigFile to true before building the override config.

This change ensures that the ESLint configuration override will be correctly applied, aligning with the changeset description.


251-252:

Details

✅ Verification successful

Consider using assignment to undefined instead of delete.

The delete operator can impact performance. Additionally, there's a non-null assertion (!) that could be avoided with a proper existence check.

- // FIXME: Seems like a bug in eslint - https://github.com/eslint/eslint/issues/19722
- delete eslintConfig.overrideConfig.plugins!['@'];
+ // FIXME: Seems like a bug in eslint - https://github.com/eslint/eslint/issues/19722
+ if (eslintConfig.overrideConfig.plugins) {
+   eslintConfig.overrideConfig.plugins['@'] = undefined;
+ }

🏁 Script executed:

#!/bin/bash
# Check if there are other places in the codebase using delete operator with plugins
rg "delete.*plugins" --type ts

Length of output: 145


To determine whether we should apply the same “delete → assignment” change to the other occurrence (delete eslintConfig.plugins;), let’s grab the surrounding context:


🏁 Script executed:

#!/bin/bash
# Show 3 lines before/after the top‐level plugins deletion for context
rg -n -A3 -B3 "delete eslintConfig.plugins;" --type ts src/index.ts

Length of output: 364


I've located where overrideConfig and its plugins property are defined; it's typed as a Record<string, Plugin>, where a missing property (undefined) is not equivalent to an absent key when ESLint processes plugins. Assigning undefined would leave the key in the object, potentially causing ESLint to attempt to load a plugin named '@' with an undefined value—unlike delete, which cleanly removes the property.

Given the semantic difference, it’s safer to keep using delete for property cleanup despite its slight performance cost, and simply avoid the non-null assertion:

- // FIXME: Seems like a bug in eslint - https://github.com/eslint/eslint/issues/19722
- delete eslintConfig.overrideConfig.plugins!['@'];
+ // FIXME: Seems like a bug in eslint - https://github.com/eslint/eslint/issues/19722
+ if (eslintConfig.overrideConfig.plugins) {
+   delete eslintConfig.overrideConfig.plugins['@'];
+ }

You can leave the top-level delete eslintConfig.plugins; as is for the same reason.

🧰 Tools
🪛 Biome (1.9.4)

[error] 252-252: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

test/index.spec.ts (5)

1-1: LGTM! Simplified ESLint disable comment.

Simplified the ESLint disable comment by removing the unnecessary sonarjs/no-commented-code rule reference.


134-134: LGTM! Changed expected output to match new behavior.

Updated the test to expect noopOutput() instead of the previous output format, correctly reflecting the changes in how ESLint configuration is handled.


190-190: LGTM! Updated expected output for globals as array test.

Changed the expected output for the "accepts config globals as array" test to use noopOutput(), which aligns with the updated implementation.


287-287: LGTM! Updated string quotes in expected output.

Changed single quotes to double quotes in the expected output string, correctly reflecting the expected formatting behavior.


527-534: LGTM! Uncommented and activated the noopOutput function.

The previously commented-out noopOutput function has been activated to support tests that now rely on this output format.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented May 16, 2025

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.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (5c03479) to head (f7c0ded).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1193   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files           2        2           
  Lines         310      311    +1     
  Branches       82       82           
=======================================
+ Hits          309      310    +1     
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the ESLint configuration and updates the test expectations to use a new noop output.

  • Update ESLint configuration in src/index.ts to set overrideConfigFile to true and use overrideConfig, including targeted deletion of a plugins entry.
  • Adjust test outputs in test/index.spec.ts by replacing calls to prettierLastOutput() with noopOutput() and updating string formatting for consistency.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/index.spec.ts Replaces expected outputs with noopOutput() and updates quotes.
src/index.ts Modifies ESLint configuration, enabling overrideConfigFile and refining plugin deletion.
Comments suppressed due to low confidence (1)

test/index.spec.ts:134

  • [nitpick] Ensure that the change in expected output from prettierLastOutput() to noopOutput() is fully covered by unit tests so that the new behavior is verified.
output: noopOutput(),

Signed-off-by: JounQin <admin@1stg.me>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented May 16, 2025

Open in StackBlitz

npm i https://pkg.pr.new/prettier-eslint@1193

commit: f7c0ded

Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 520166c in 2 minutes and 5 seconds. Click for details.
  • Reviewed 84 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 draft 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:231
  • Draft comment:
    Mutating the passed eslintConfig by setting 'overrideConfigFile = true' could have side effects if the config is reused. Consider cloning the config before modification.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
2. src/index.ts:252
  • Draft comment:
    Using 'delete eslintConfig.overrideConfig.plugins!['@']' with a non‐null assertion and hardcoded key may be fragile. Consider checking if the property exists before deletion.
  • 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 is a workaround for a specific ESLint bug. Looking at line 237, plugins is explicitly set to {} if undefined, so plugins will never be null here. The non-null assertion is actually correct in this case. The optional chaining suggestion would add unnecessary safety checks for a condition we know can't happen. The comment raises a valid point about code fragility in general. Optional chaining is often safer than non-null assertions. While optional chaining is generally good practice, in this specific case the non-null assertion is correct because we can prove plugins will never be null due to line 237's default value assignment. The comment should be deleted because it suggests a change that would add unnecessary safety checks in code where we can prove the value is never null.
3. test/index.spec.ts:1
  • Draft comment:
    Removal of the 'sonarjs/no-commented-code' lint rule is minor, but ensure it’s intentional to allow commented-out code for tracking known issues.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
4. test/index.spec.ts:131
  • Draft comment:
    Expected output changed from 'prettierLastOutput()' to 'noopOutput()'. Confirm that this matches the new intended behavior.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
5. test/index.spec.ts:187
  • Draft comment:
    Similarly, expected output is now 'noopOutput()' instead of 'prettierLastOutput()' for the globals test. Verify that the change is correct.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
6. test/index.spec.ts:284
  • Draft comment:
    The expected CSS output now uses double quotes instead of single quotes. Ensure that this formatting change is consistent with the configured rules.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
7. test/index.spec.ts:527
  • Draft comment:
    The previously commented-out noopOutput function is now active. Confirm that its returned output exactly matches the expected result in tests.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_aUV3TU0aRKEqgHMy

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@JounQin
Copy link
Copy Markdown
Member Author

JounQin commented May 16, 2025

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.

@coderabbitai We're using yarn v4.

@JounQin JounQin merged commit 0bd2e77 into master May 16, 2025
17 checks passed
@JounQin JounQin deleted the fix/overrideConfig branch May 16, 2025 10:25
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented May 16, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@JounQin Thanks for letting me know! The error is occurring because CodeRabbit's environment uses an older version of Yarn that doesn't support Yarn v4's portal references (portal:.). Since your project is correctly configured for Yarn v4, rather than changing your project setup, you should disable the ESLint tool in CodeRabbit's configuration.

You can do this by creating or updating a .coderabbit.yaml file in your repository with:

# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
tools:
  eslint:
    enabled: false

This will prevent CodeRabbit from trying to run ESLint during reviews while keeping all other code review functionality intact. The warning will no longer appear in future PRs.


🏁 Script executed:

#!/bin/bash
# Check package.json for dependency configuration
cat package.json

Length of output: 3158


@JounQin Thanks for sharing that information. After examining your package.json, I can confirm that the issue is due to the Yarn v4 feature you're using:

"devDependencies": {
  "prettier-eslint": "portal:.",
},
"resolutions": {
  "@prettier/eslint": "portal:.",
}

The portal:. protocol is specific to Yarn v4 and not supported by CodeRabbit's environment, which is using Yarn v1.22.22. That's why you're seeing:

Couldn't find any versions for "prettier-eslint" that matches "portal:."

Since this is a proper setup for your project's development workflow and you shouldn't have to change it, the best solution is to disable the ESLint tool in CodeRabbit. You can do this by creating or updating a .coderabbit.yaml file in your repository:

# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
tools:
  eslint:
    enabled: false

This will prevent these warnings in future PR reviews while keeping all other code review functionality intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants