@snitin315 Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment that I will keep updated.

1 package in this PR

• eslint (probably deleted!) — on npm, on unpkg
  • Config files to check:
    • eslint/.eslintrc.json: edited
    • eslint/v8/.eslintrc.json: edited

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ A DT maintainer needs to approve changes that affect module config files

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

{
  "type": "info",
  "now": "-",
  "pr_number": 70735,
  "author": "snitin315",
  "headCommitOid": "25a0a797d2bf24498119dd6e281d33b391c71add",
  "mergeBaseOid": "29a5cdc0807e42681cf7cf300bd49ab865a77d9b",
  "lastPushDate": "2024-09-29T11:32:18.000Z",
  "lastActivityDate": "2024-10-16T16:12:03.000Z",
  "mergeOfferDate": "2024-10-16T13:44:12.000Z",
  "mergeRequestDate": "2024-10-16T16:12:03.000Z",
  "mergeRequestUser": "snitin315",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": true,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "eslint",
      "kind": "delete",
      "files": [
        {
          "path": "types/eslint/.eslintrc.json",
          "kind": "package-meta",
          "suspect": "edited"
        },
        {
          "path": "types/eslint/.npmignore",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/eslint-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/eslint/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/rules/best-practices.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/deprecated.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/ecmascript-6.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/node-commonjs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/possible-errors.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/strict-mode.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/stylistic-issues.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/rules/variables.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/tsconfig.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/use-at-your-own-risk.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/.eslintrc.json",
          "kind": "package-meta",
          "suspect": "edited"
        },
        {
          "path": "types/eslint/v8/.npmignore",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/v8/eslint-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/eslint/v8/helpers.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/v8/rules/best-practices.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/deprecated.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/ecmascript-6.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/node-commonjs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/possible-errors.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/strict-mode.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/stylistic-issues.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/rules/variables.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/eslint/v8/tsconfig.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/eslint/v8/use-at-your-own-risk.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "pmdartus",
        "j-f1",
        "saadq",
        "JasonHK",
        "bradzacher",
        "JounQin",
        "bmish"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "sandersn",
      "date": "2024-10-16T13:43:28.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "bradzacher",
      "date": "2024-10-10T13:57:39.000Z",
      "abbrOid": "d4dc8eb"
    },
    {
      "type": "stale",
      "reviewer": "JoshuaKGoldberg",
      "date": "2024-09-29T13:04:38.000Z",
      "abbrOid": "d4dc8eb"
    }
  ],
  "mainBotCommentID": 2381321344,
  "ciResult": "pass"
}

🔔 @pmdartus @j-f1 @saadq @JasonHK @bradzacher @JounQin @bmish — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@JoshuaKGoldberg Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@types/eslint has so many dependents that I'd recommend skipping the publish of the deprecated stub. Especially old types dependents like eslint-plugin-markdown will break when they get the stub. Not a (big) problem if people have upgraded to an eslint with types, since they'll get those instead. But lots of projects haven't upgraded yet.

To skip the stub publish, revert the addition to notNeededPackage.json.

Re-ping @pmdartus, @j-f1, @saadq, @JasonHK, @bradzacher, @JounQin, @bmish:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

@sandersn Done.

@bradzacher, @JoshuaKGoldberg Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

Nice, thanks for this @snitin315 and others involved!

What happens with the existing @types/eslint package on npm now?

It is currently still published at version 9.6.1 (last publish: 26 Aug 2024) without any warnings or deprecation notices:

Or maybe there are plans already which I missed to manually deprecate this from npm...?

@karlhorky I believe usually after some time the DT bot deprecates the package and adds the warning This is a stub types definition. eslint provides its own type definitions, so you do not need this installed.. Maybe this was granted a special exception as it is such a popular types package?

That's right. My explanation is at #70735 (comment)

@sandersn oh, yeah I saw that comment.

Does that mean that the obsolete @types/eslint package will stay online and published in the future without any notice to users?

If yes, seems like this will lead to confusion for new projects and existing projects, and unnecessary dependencies being installed...?

@karlhorky @sandersn Yeah I agree, I think instead of not publishing a stub and not publishing a deprecation notice, we should only skip the stub, but keep the notice.

This shouldn't break anything, unless for some reason they have a fail on deprecation notice option enabled.

Maybe even a custom deprecation notice, linking to an explaination page, seeing how big this dependency is? @types/eslint is no longer maintained or up to date with ESLint's types, as it includes its own. See https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70735

We definitely shouldn't stub the types though, that would break a lot of projects.

@karlhorky @types/eslint stays online with the last version at 9.6.1 -- no more publishes.

@pauliesnug I like the idea of a custom deprecate notice, but there's no place to put it unless we publish another version of @types/eslint -- [email protected] doesn't ship types, so it makes sense for people to install @types/[email protected]. I guess we could re-publish 9.6.1 manually as 9.10.0, but that seems misleading as well.

I like the idea of a custom deprecate notice, but there's no place to put it unless we publish another version of @types/eslint

What about npm deprecate? That wouldn't require publishing another version I guess, would it?

What I mean is, npm deprecate @types/[email protected] doesn't make sense, because [email protected] doesn't ship types. So @types/[email protected] shouldn't be deprecated. It still makes sense to install.

Ok, I think I'm understanding what you mean.

To be clear, what I think could be a common user flow:

1. User is setting up a new project with ESLint, and relies on their memory (or by looking at a project they set up previously)
2. User remembers that for ESLint, the types are not included and @types/eslint is required
3. User checks npm page @types/eslint to be sure (does not look at version number)
4. User does not see any deprecation notice or anything that says that @types/eslint is obsolete now (does not go looking through old closed issues / PRs to investigate)
5. 💥 User installs @types/eslint along with ESLint - an obsolete, unnecessary package

If @types/eslint is not stubbed (maybe it should actually be stubbed after all? if not right away now, maybe when ESLint v9 becomes end-of-life?), then it seems like it should at least have a notice showing that it is not recommended to install with the latest version of ESLint.

The bad thing that happens at (5) is not a crash, it's an extra 253 KB in node_module that the compiler ignores, plus an unnecessary entry in package.json. It is bad that the JS ecosystem continues to accrue cruft.

Also, deprecating stub packages got even less useful recently with npm/npm-pick-manifest#33; npm now never resolves to a deprecated package when any un-deprecated one exists. So npm i @types/eslint would always give 9.6, not the 9.10 stub. Maybe what we could do is publish a final patch release, 9.6.2, that's a copy of 9.6.1 with an updated README that adds "if you're using a version of eslint 9.10 or newer, you don't need these types." That would stop your scenario at (3).

However, I don't think most people will npm i @types/[email protected] or check the types README, so neither will reach the majority of people.

How about releasing a stub @types/[email protected] package? Version 9.6.1 (or 9.6.2) will be the last un-deprecated package with actual typings in place.

#70735 (comment). Briefly, types packages depend on each other with *, so they'll get the stub instead of working types -- at least on old versions of npm. Newer versions of npm will resolve to a working version. But...so will people when they npm i @types/eslint.

The flip side is that without the stub, all packages on DT will only ever test the DT package, because they'll never know to download eslint instead. That's basically what happened when this PR was merged, so I'm not sure that's a good result either... This hole didn't exist before we switched to a real pnpm monorepo, because DT would not have been able to pull older types from npm without interaction. Things could be very broken right now with people using eslint directly and we'd have no idea.

I recently merged in support for peer deps; given eslint packages are extensions of eslint and therefore would be peer deps, perhaps we should fix all of those to be peer deps on @types/eslint and then publish the stub? That way, a user can fairly easily install the "right" types?

(But, doing so will forcibly install @types/eslint even when it's not needed due to it being eslint v9, so, it's all terrible. That's not different than today, though.)