Skip to content

Conversation

@lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Oct 30, 2025

Prerequisites checklist

What is the purpose of this pull request?

This PR provides one possible fix for #308.

It partially reverts commit 02bac50.


I'm not entirely sure about this fix for the following reason:

  • @eslint/core is in a 0.x release, so these breakages may be acceptable.
  • Technically, making fix?: RuleFixer | null | undefined; required under suggest wouldn't cause a problem, but I think the SuggestedEditBase type is used incorrectly somewhere, which is causing the issue.

I believe there may be a better solution, so please consider this PR a hotfix for the problem. (Or any other suggestions would be welcome.)

What changes did you make? (Give an overview)

This PR provides one possible fix for #308.

It partially reverts commit 02bac50.

Related Issues

Closes: #308

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

N/A

@eslint-github-bot

This comment was marked as resolved.

@lumirlumir lumirlumir changed the title revert: "fix: require fix in suggestion objects (#298)" fix: revert "fix: require fix in suggestion objects (#298)" Oct 30, 2025
@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Oct 30, 2025
@lumirlumir lumirlumir changed the title fix: revert "fix: require fix in suggestion objects (#298)" fix: partial revert "fix: require fix in suggestion objects (#298)" Oct 30, 2025
@lumirlumir lumirlumir marked this pull request as ready for review October 30, 2025 07:12
@lumirlumir lumirlumir changed the title fix: partial revert "fix: require fix in suggestion objects (#298)" fix: partially revert "fix: require fix in suggestion objects (#298)" Oct 30, 2025
@lumirlumir lumirlumir marked this pull request as draft October 30, 2025 07:41
@lumirlumir
Copy link
Member Author

Pending verification of #308 (comment).

@fasttime fasttime moved this from Needs Triage to Triaging in Triage Oct 30, 2025
@fasttime
Copy link
Member

I'm not sure if it's necessary to merge this fix because this would revert the types to a less correct stand. I think the build is failing because we didn't do a minor release @eslint/[email protected] for the changes in 02bac50. Instead, everything was released in @eslint/[email protected], which npm treats as a a major release and that means that npm cannot pick it automatically when it resolves the dependencies for eslint. So eslint it continues to depend on @eslint/[email protected] and that is where the inconsistency arises:

$ ...\rewrite> npm list @eslint/core
[email protected] ...\rewrite
├─┬ @eslint/[email protected] -> .\packages\compat
│ └── @eslint/[email protected] deduped -> .\packages\core
├─┬ @eslint/[email protected] -> .\packages\config-helpers
│ └── @eslint/[email protected] deduped -> .\packages\core
├── @eslint/[email protected] -> .\packages\core
├─┬ @eslint/[email protected] -> .\packages\migrate-config
│ └── @eslint/[email protected] deduped -> .\packages\core
├─┬ @eslint/[email protected] -> .\packages\plugin-kit
│ └── @eslint/[email protected] deduped -> .\packages\core
└─┬ [email protected]
  └── @eslint/[email protected]

Another way to fix the build failure is to use an overrides in package.json make sure that eslint uses the local version of @eslint/core when it runs in this repo:

"overrides": {
  "eslint": {
    "@eslint/core": "file:packages/core"
  }
}

@lumirlumir lumirlumir changed the title fix: partially revert "fix: require fix in suggestion objects (#298)" chore: fix CI failure and ensure the latest @eslint/core is used Oct 30, 2025
@lumirlumir
Copy link
Member Author

@fasttime Thanks for the solution and guidance!

I've updated this PR according to your comment, and I've also updated the PR title to reflect the change.

@lumirlumir lumirlumir marked this pull request as ready for review October 30, 2025 10:45
@fasttime fasttime moved this from Triaging to Implementing in Triage Oct 30, 2025
Copy link
Member

@fasttime fasttime 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!

@fasttime fasttime merged commit 77442ae into main Oct 30, 2025
25 checks passed
@fasttime fasttime deleted the revert branch October 30, 2025 11:51
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 chore

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: All downstream version-bump builds are failing after the rewrite was released

3 participants