Skip to content

Conversation

@hyperz111
Copy link
Contributor

@hyperz111 hyperz111 commented Oct 13, 2025

Prerequisites checklist

What is the purpose of this pull request?

Improve shareable config (--config flag). Throw if not found, and can use package tag (e.g. my-eslint-config@alpha).

What changes did you make? (Give an overview)

Example: esl-con-rec

Before:

$ node bin/create-config.js --config esl-con-rec
@eslint/create-config: v1.10.0

ℹ The config that you've selected requires the following dependencies:

eslint, esl-con-rec, error@[object Object], @eslint/config-helpers
? Would you like to install them now? › No / Yes

After:

$ node bin/create-config.js --config esl-con-rec
@eslint/create-config: v1.10.0

file:///data/data/com.termux/files/home/gitclone/eslint-create-config/bin/create-config.js:24
        throw error;
                ^

Error: Cannot fetch "esl-con-rec" with error: Not found
    at fetchPeerDependencies (file:///data/data/com.termux/files/home/gitclone/eslint-create-config/lib/utils/npm-utils.js:134:15)
    at ConfigGenerator.calc (file:///data/data/com.termux/files/home/gitclone/eslint-create-config/lib/config-generator.js:294:33)
    at file:///data/data/com.termux/files/home/gitclone/eslint-create-config/bin/create-config.js:52:21

Node.js v24.7.0

Related Issues

N/A

Is there anything you'd like reviewers to focus on?

N/A

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Oct 13, 2025
@eslintbot eslintbot added this to Triage Oct 13, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 13, 2025
@aladdin-add aladdin-add self-requested a review October 13, 2025 04:31
@aladdin-add aladdin-add self-assigned this Oct 13, 2025
@aladdin-add aladdin-add moved this from Needs Triage to Implementing in Triage Oct 13, 2025
@hyperz111 hyperz111 changed the title fix: improve shareable config feat: improve shareable config Oct 13, 2025
@hyperz111
Copy link
Contributor Author

Maybe this is can be a feature instead a fix

@aladdin-add aladdin-add requested a review from Copilot October 14, 2025 01:37
Copy link

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 PR improves the shareable config functionality by making the system more robust when handling missing or invalid packages, and adds support for package tags. The key changes include switching from npm CLI commands to direct registry API calls and implementing proper error handling.

Key changes:

  • Replaces npm CLI dependency with direct npm registry API calls for fetching peer dependencies
  • Implements proper error handling that throws exceptions for missing packages instead of silently returning null
  • Adds support for package tags using semver resolution

Reviewed Changes

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

File Description
lib/utils/npm-utils.js Refactored fetchPeerDependencies to use npm registry API directly with proper error handling and semver tag support
lib/config-generator.js Updated to handle the new error-throwing behavior and extract package names properly
tests/utils/npm-utils.spec.js Updated tests to reflect the new API-based approach and error-throwing behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

`Version "${version}" not found for package "${name}".`
);
}
return Object.entries(Object(packageVersion.peerDependencies)).map(
Copy link
Member

Choose a reason for hiding this comment

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

why Object() is used here? Is this to convert packageVersion.peerDependencies to {} when it is undefined?

stub.resolves(mockResponse);

assert.isNull(peerDependencies);
expect(async () => await fetchPeerDependencies("desired-package")).rejects.toThrowError();
Copy link
Member

Choose a reason for hiding this comment

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

can we use assert.throws() here? For consistency - the other tests are all assert-style.

@hyperz111
Copy link
Contributor Author

@aladdin-add, this is ok?

@aladdin-add aladdin-add self-requested a review October 16, 2025 03:12
@hyperz111
Copy link
Contributor Author

@aladdin-add, ping


const error = npmProcess.error;
try {
// eslint-disable-next-line n/no-unsupported-features/node-builtins -- Fallback using built-in fetch
Copy link
Member

Choose a reason for hiding this comment

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

the comment needs an update. :)

@hyperz111
Copy link
Contributor Author

Maybe i should add test for package with version tag.

@aladdin-add
Copy link
Member

Maybe i should add test for package with version tag.

👍

@aladdin-add
Copy link
Member

when running --config eslint-config-eslint-x, it output:

PS D:\repos\eslint-org\create-config> node .\bin\create-config.js --config eslint-config-eslint-x    
@eslint/create-config: v1.10.0

file:///D:/repos/eslint-org/create-config/bin/create-config.js:24
        throw error;
        ^

Error: Cannot fetch "eslint-config-eslint-x@latest" with error: Unexpected Error when fetching package `e
slint-config-eslint-x`: Not found                                                                            at fetchPeerDependencies (file:///D:/repos/eslint-org/create-config/lib/utils/npm-utils.js:114:15)   
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
    at async ConfigGenerator.calc (file:///D:/repos/eslint-org/create-config/lib/config-generator.js:295:
27)                                                                                                          at async file:///D:/repos/eslint-org/create-config/bin/create-config.js:52:5

Node.js v25.0.0

the error message seems a bit strange.

@hyperz111
Copy link
Contributor Author

when running --config eslint-config-eslint-x, it output:

PS D:\repos\eslint-org\create-config> node .\bin\create-config.js --config eslint-config-eslint-x    
@eslint/create-config: v1.10.0

file:///D:/repos/eslint-org/create-config/bin/create-config.js:24
        throw error;
        ^

Error: Cannot fetch "eslint-config-eslint-x@latest" with error: Unexpected Error when fetching package `e
slint-config-eslint-x`: Not found                                                                            at fetchPeerDependencies (file:///D:/repos/eslint-org/create-config/lib/utils/npm-utils.js:114:15)   
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
    at async ConfigGenerator.calc (file:///D:/repos/eslint-org/create-config/lib/config-generator.js:295:
27)                                                                                                          at async file:///D:/repos/eslint-org/create-config/bin/create-config.js:52:5

Node.js v25.0.0

the error message seems a bit strange.

Maybe you're right, so what the Error message we should throw?

@aladdin-add
Copy link
Member

Cannot fetch "eslint-config-eslint-x@latest" with error: Unexpected Error when fetching package `e
slint-config-eslint-x`

maybe we could revert the change: 122699f?

@aladdin-add
Copy link
Member

could you also checkout some unresolved comments, otherwise LGTM, thanks! 👍

Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

💯 LGTM, thanks! leaving it open for 2~3 days for others to review.

@aladdin-add aladdin-add moved this from Implementing to Second Review Needed in Triage Oct 20, 2025
@aladdin-add aladdin-add merged commit 3fccf98 into eslint:main Oct 22, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Oct 22, 2025
@github-actions github-actions bot mentioned this pull request Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted bug Something isn't working feature

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

2 participants