Skip to content

fix: add missing .d.cts files while building plugin-kit#445

Merged
lumirlumir merged 5 commits into
eslint:mainfrom
liangmiQwQ:fix/add-dts-for-plugin-kit
May 19, 2026
Merged

fix: add missing .d.cts files while building plugin-kit#445
lumirlumir merged 5 commits into
eslint:mainfrom
liangmiQwQ:fix/add-dts-for-plugin-kit

Conversation

@liangmiQwQ
Copy link
Copy Markdown
Contributor

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What is the purpose of this pull request?

Please read sxzz/rolldown-plugin-dts#1230. ESLint should emit .d.ts for commonjs for downstream's build.

What changes did you make? (Give an overview)

Copied core package's bundling logic

Related Issues

sxzz/rolldown-plugin-dts#1230

@github-project-automation github-project-automation Bot moved this to Needs Triage in Triage May 3, 2026
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 3, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@eslint-github-bot
Copy link
Copy Markdown

Hi @liangmiQwQ!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
  • There should be a space following the initial tag and colon, for example 'feat: Message'.

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

@liangmiQwQ liangmiQwQ changed the title fix(plugin-kit): add missing .d.cts files while building fix: add missing .d.cts files while building plugin-kit May 3, 2026
@eslint-github-bot eslint-github-bot Bot added the bug Something isn't working label May 3, 2026
@liangmiQwQ

This comment was marked as resolved.

@DMartens
Copy link
Copy Markdown

DMartens commented May 3, 2026

I cannot reproduce this problem as dist/cjs/index.d.cts for plugin-kit is created.
You can see this in the package.json script build:cts you want to modify.
Regarding the EasyCLA problem, please retry maybe in a different browser, as we cannot proceed otherwise.

@DMartens DMartens moved this from Needs Triage to Triaging in Triage May 3, 2026
@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

liangmiQwQ commented May 3, 2026

I cannot reproduce this problem as dist/cjs/index.d.cts for plugin-kit is created. You can see this in the package.json script build:cts you want to modify. Regarding the EasyCLA problem, please retry maybe in a different browser, as we cannot proceed otherwise.

I want to point out the file we need is the type.d.ts, which is missing in cjs directory. I think we should just do the things ESlint has already done in core package instead of modifying or calling the global building script (as there are no other imports in type.cts).

"build:cts": "node -e \"fs.cpSync('dist/esm/types.d.ts', 'dist/cjs/types.d.cts')\"",

And I will try to solve EasyCLA asap. Thank you.

Update: EasyCLA was solved.

@liangmiQwQ liangmiQwQ force-pushed the fix/add-dts-for-plugin-kit branch from 171efba to 749d75f Compare May 3, 2026 14:24
Copy link
Copy Markdown
Member

@lumirlumir lumirlumir left a comment

Choose a reason for hiding this comment

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

Actually, it’s a bit difficult to pinpoint the root cause here, since your reproduction link is about the rolldown integration.

Could you please share a failing minimal reproduction link without the rolldown integration?

Also, updating the PR template to use our bug template would be helpful here, so others can more easily recognize the real problem.

@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

liangmiQwQ commented May 4, 2026

Could you please share a failing minimal reproduction link without the rolldown integration?

Umm... this might be a little bit hard to describe or be shown in a reproduction repo, I think I need to re-explain the question.

@eslint/plugin-kit exposes index.d.(c)ts for both esm and cjs, which imports types.(c)ts. The bundler will automatically redirects all pure .(c)ts files to .d.(c)ts as a type declaration (most of the bundlers do it, like rollup, as well as rolldown).

And for the current @eslint/plugin-kit's building process, the types.d.ts file only exists under esm directory, not cjs directory, which causes bundler can't find the target .d.cts when using CommonJS.

So, for packages who depend on and bundle @eslint/plugin-kit.

  • If they are using Rollup, it will automatically generate a types.d.cts as it is missing.
  • If they are using rolldown (like above), it won't generate this file, make bundling error happens.

I've talked with rolldown team, they thought it is expected and it's an ESLint's side building config issue. It makes sense, because the esm side has types.d.ts (Generated by tsc), but the cjs has no types.d.cts, which is missing exactly.

Hope it's helpful, thanks.

@DMartens
Copy link
Copy Markdown

DMartens commented May 4, 2026

Thank you for the explanation.
I think that this is not bundler-specific problem and agree that this should be fixed on our side.
The changes make sense to me, leaving open for @lumirlumir.

@DMartens DMartens moved this from Triaging to Implementing in Triage May 4, 2026
@liangmiQwQ liangmiQwQ requested a review from lumirlumir May 4, 2026 23:17
@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

Thank You!

@github-actions
Copy link
Copy Markdown
Contributor

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

@github-actions github-actions Bot added the Stale label May 15, 2026
@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

Hello @lumirlumir, sorry for pinging, but it seems we need to make it active!

@github-actions github-actions Bot removed the Stale label May 16, 2026
@lumirlumir
Copy link
Copy Markdown
Member

@liangmiQwQ

Apologies for the delay. I've been a bit busy lately, and I'll make sure to review this change by tomorrow. Thanks in advance for your patience!

@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

Never mind!

However, I'm not sure whether the CI falling is related to my changes in 501d4ad.

@lumirlumir lumirlumir moved this from Implementing to Second Review Needed in Triage May 19, 2026
Copy link
Copy Markdown
Member

@lumirlumir lumirlumir 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 for the time you spent waiting.

I like this change, and I’m leaving some notes here for future reference.

After looking into this a bit more, I don’t think the missing dist/cjs/types.d.cts is a problem for standard TypeScript type resolution by itself. dist/cjs/types.cts is not exposed as a public API entry point and is only referenced from index.d.cts, which TypeScript can resolve correctly.

So I understand this change mainly as a packaging compatibility improvement for downstream declaration-bundling tools, such as rolldown/rolldown-plugin-dts, which may not re-emit declarations for files inside node_modules. Shipping the CJS declaration graph in a complete .d.cts form makes the package more robust for those integrations and will not affect ordinary users.

@lumirlumir
Copy link
Copy Markdown
Member

However, I'm not sure whether the CI falling is related to my changes in 501d4ad.

I think that was a temporary issue from a downstream dependency and was fixed in a separate PR, so no worries at all!

@lumirlumir lumirlumir merged commit 49e101b into eslint:main May 19, 2026
35 checks passed
@github-project-automation github-project-automation Bot moved this from Second Review Needed to Complete in Triage May 19, 2026
@eslintbot eslintbot mentioned this pull request May 19, 2026
@liangmiQwQ liangmiQwQ deleted the fix/add-dts-for-plugin-kit branch May 19, 2026 14:22
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

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants