prettier/eslint-config-prettierPublic

NotificationsYou must be signed in to change notification settings
Fork264
Star5.8k

fix(cli): do not crash on no rules configured#328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

JounQin merged 5 commits intoprettier:mainfromsilvenon:handle-empty-flat-config

May 9, 2025

Merged

fix(cli): do not crash on no rules configured#328

JounQin merged 5 commits intoprettier:mainfromsilvenon:handle-empty-flat-config

May 9, 2025

+24 −2

Conversation

Copy link

Contributor

silvenon commentedMay 7, 2025•
edited by coderabbitaibot
Loading

In the case when a flat config has no rules defined, the resolved config for a file isundefined.

// eslint.config.jsexportdefault[{}]

This causes the CLI utility to fail to destructurerules out of it:

❯ npx eslint-config-prettier src/utils/toast.tsxCannot destructure property 'rules' of 'undefined' as it is undefined.

and it's also necessary to initializerules as well, becauseObject.entries(undefined) is an error.

This will achieve the same behavior as a legacy configuration:

// .eslintrc.cjsmodule.exports={}

❯ npx eslint-config-prettier src/utils/toast.tsxNo rules that are unnecessary or conflict with Prettier were found.

Fixes#288.

Important

Fixes CLI crash inbin/cli.js by setting defaultrules = {} for flat configs with no rules, ensuring consistent behavior with legacy configurations.

Behavior:
- Fixes CLI utility crash inbin/cli.js when flat config has no rules defined by setting defaultrules = {} during destructuring.
- Ensures consistent behavior with legacy configuration by outputting "No rules that are unnecessary or conflict with Prettier were found." when no rules are present.
Misc:
- Fixes[CLI] Handle undefined computed config #288.

^{This description was created by}^{for0b82629. You cancustomize this summary. It will automatically update as commits are pushed.}

Summary by CodeRabbit

Bug Fixes
- Improved stability by preventing potential errors when configuration objects or their properties are missing.
- Ensured the CLI runs smoothly even with empty or missing configuration files.

Fix case in flat config when a file doesn't have any rules configured

0b82629

In flat config the resolved config is "undefined" for files that don'thave any configuration, which caused an error in the CLI utility, unlikein legacy ESLint configuration where the empty resolved configuration isan empty object with an empty "rules: {}".

Copy link

changeset-botbot commentedMay 7, 2025•
edited
Loading

🦋 Changeset detected

Latest commit:cd29dcb

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

This PR includes changesets to release 1 package

Name	Type
eslint-config-prettier	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

Copy link

coderabbitaibot commentedMay 7, 2025•
edited
Loading

Walkthrough

The change updates the destructuring of therules property from configuration objects within the CLI script. It now safely handles cases where the configuration object or itsrules property might be undefined by providing default empty objects, preventing runtime errors when accessingrules on undefined configurations. A new test verifies the CLI runs without error when given an empty config.

Changes

File(s)	Change Summary
bin/cli.js	Modified destructuring of`rules` from config objects to provide default empty objects for safety.
test/cli.test.js	Added a test case`"empty config"` that runs the CLI against an empty config fixture to ensure no runtime error.
test/fixtures/empty-config/eslint.config.mjs	Added a new empty ESLint config exporting a default array with a single empty object.
.changeset/lazy-dingos-cheat.md	Added a changeset documenting a patch update fixing CLI crash when no rules are configured.

Assessment against linked issues

Objective	Addressed	Explanation
Handle undefined computed config in CLI, ensuring no runtime failure (#288)	✅

Poem

A hop and a skip, with configs in tow,
No more undefined, the errors won't show.
With empty objects, we safely proceed,
The CLI now handles each config's need.
🐇✨ Bugs are out, and all is right—
Hopping along with code so bright!

Tip

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configureReview - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off theData Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweenbaa5e3d andcd29dcb.

📒 Files selected for processing (1)

.changeset/lazy-dingos-cheat.md (1 hunks)

✅ Files skipped from review due to trivial changes (1)

.changeset/lazy-dingos-cheat.md

⏰ Context from checks skipped due to timeout of 90000ms (3)

GitHub Check: Test on windows-latest with Node.js 22
GitHub Check: Test on windows-latest with Node.js 18
GitHub Check: Test on windows-latest with Node.js 16

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 withCodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- 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 generate unit testing code for this file.
- @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 generate unit testing code.
- @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 oursupport 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 togenerate 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 theconfiguration 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 ourDocumentation for detailed information on how to use CodeRabbit.
Join ourDiscord Community to get help, request features, and share feedback.
Follow us onX/Twitter for updates and announcements.

ellipsis-devbot reviewed

May 7, 2025

View reviewed changes

Copy link

ellipsis-devbot 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 to0b82629 in 30 seconds. Click for details.

Reviewed13 lines of code in1 files
Skipped0 files when reviewing.
Skipped posting2 draft comments. View those below.
Modify yoursettings andrules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1.bin/cli.js:63

Draft comment:
Good use of destructuring with a default. This change ensures that if a config is undefined, 'rules' defaults to an empty object, avoiding errors when calling Object.entries().
Reason this comment was not posted:
Confidence changes required:0% <= threshold50% None

2.bin/cli.js:63

Draft comment:
Good fix: defaulting 'rules' to {} prevents the destructuring error. Consider adding an inline comment referencing issue#288 to document why we use this fallback.
Reason this comment was not posted:
Confidence changes required:0% <= threshold50% None

Workflow ID:wflow_p2ecqH4CC3QoYznk

^{You can customize}^{by changing yourverbosity settings, reacting with 👍 or 👎,replying to comments, or addingcode review rules.}

Copy link

ContributorAuthor

silvenon commentedMay 7, 2025•
edited
Loading

I didn't find an (easy) way to incorporate this into tests, I would probably have to mock config files, not sure if it's worth it. Let me know if I missed something.

Copy link

Member

JounQin commentedMay 7, 2025

I didn't find an (easy) way to incorporate this into tests, I would probably have to mock config files, not sure if it's worth it. Let me know if I missed something.

An e2e test case could be added? And some comments could be added at least.

Copy link

ContributorAuthor

silvenon commentedMay 8, 2025

Since running the CLI doesn't allow me to specify the config, what kind of an E2E test did you have in mind? What I can do is pass a non-JS file likefoo. Legacy configs will still give those the their default config when runningnpx eslint --print-config foo, while flat configs will be much more strict and apply only to JS extensions.

It would pick up your project'seslint.config.js, but it would still outputundefined, which is what's needed. Would that be good enough?

Copy link

Member

JounQin commentedMay 8, 2025

exec in custom fixture cwd?

silvenon added2 commits

May 8, 2025 10:18

chore: add comment for workaround for no rules in flat config

b03296c

test: add test for unmatched flat config case

2dc0061

Copy link

ContributorAuthor

silvenon commentedMay 8, 2025

Great idea, done ✅

coderabbitaibot reviewed

May 8, 2025

View reviewed changes

Copy link

coderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/cli.test.js (1)
81-87:Good addition of a test case for empty config scenario.
This test effectively verifies that the CLI doesn't crash when encountering an empty configuration, which directly addresses issue#288 mentioned in the PR objectives. The test runs the CLI script as a child process against a fixture with an empty config and implicitly checks that no error occurs.
A few suggestions to strengthen this test:
Consider asserting the expected CLI output (should be "No rules that are unnecessary or conflict with Prettier were found.")
Add a comment explaining what the test is checking and its relationship to issue#288
test("empty config", (done) => {+  // Verifies that the CLI doesn't crash with empty configs (issue #288)+  // The fixture contains an eslint.config.mjs that exports [{}]  childProcess.exec(    `node ${path.join(process.cwd(), "bin/cli.js")} index.js`,    { cwd: path.join(__dirname, "fixtures/empty-config") },-    done+    (error, stdout) => {+      expect(error).toBeNull();+      expect(stdout.trim()).toBe("No rules that are unnecessary or conflict with Prettier were found.");+      done();+    }  );});

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between0b82629 and2dc0061.

📒 Files selected for processing (3)

bin/cli.js (1 hunks)
test/cli.test.js (2 hunks)
test/fixtures/empty-config/eslint.config.mjs (1 hunks)

✅ Files skipped from review due to trivial changes (1)

test/fixtures/empty-config/eslint.config.mjs

🚧 Files skipped from review as they are similar to previous changes (1)

bin/cli.js

⏰ Context from checks skipped due to timeout of 90000ms (4)

GitHub Check: Test on windows-latest with Node.js 22
GitHub Check: Test on windows-latest with Node.js 20
GitHub Check: Test on windows-latest with Node.js 18
GitHub Check: Test on windows-latest with Node.js 16

JounQin reviewed

May 8, 2025

View reviewed changes

test/fixtures/empty-config/eslint.config.mjsShow resolvedHide resolved

JounQin requested changes

May 9, 2025

View reviewed changes

test/cli.test.js OutdatedShow resolvedHide resolved

Apply suggestions from code review

baa5e3d

JounQin changed the title~~[CLI]: Fix case in flat config when a file doesn't have any rules configured~~fix(cli): do not crash on no rules configured

May 9, 2025

Create lazy-dingos-cheat.md

cd29dcb

JounQin approved these changes

May 9, 2025

View reviewed changes

JounQin requested a review fromCopilot

May 9, 2025 01:59

CopilotAI reviewed

May 9, 2025

View reviewed changes

Copy link

CopilotAI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

JounQin added the enhancement label

May 9, 2025

JounQin requested a review fromCopilot

May 9, 2025 02:01

CopilotAI reviewed

May 9, 2025

View reviewed changes

Copy link

CopilotAI 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 PR fixes a bug in the CLI where an empty flat configuration (i.e., one with no rules specified) mistakenly caused a crash.

Fixes CLI crash by defaulting missing rules to an empty object
Adds a test to verify CLI behavior with an empty configuration
Updates changeset metadata for the patch release

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File	Description
test/fixtures/empty-config/eslint.config.mjs	Provides an empty configuration for testing
test/cli.test.js	Adds a test verifying the CLI does not crash with empty config
.changeset/lazy-dingos-cheat.md	Documents the patch release for this bug fix

Comments suppressed due to low confidence (1)

test/cli.test.js:82

Consider asserting the output message or exit code in the empty config test to ensure that the CLI behavior matches the expected "No rules that are unnecessary or conflict with Prettier were found." message.

test("empty config", (done) => {

JounQin merged commit94b4799 intoprettier:main

May 9, 2025

14 checks passed

github-actionsbot mentioned this pull request

May 9, 2025

chore: release eslint-config-prettier#329

Merged

silvenon deleted the handle-empty-flat-config branch

May 9, 2025 07:32