Skip to content

feat: add type definitions#246

Merged
MichaelDeBoey merged 8 commits into
eslint-community:mainfrom
aryaemami59:type-definitions
Jan 14, 2026
Merged

feat: add type definitions#246
MichaelDeBoey merged 8 commits into
eslint-community:mainfrom
aryaemami59:type-definitions

Conversation

@aryaemami59

Copy link
Copy Markdown

This PR:

@aryaemami59 aryaemami59 force-pushed the type-definitions branch 2 times, most recently from 5826543 to d8f9980 Compare October 26, 2024 21:56
@aryaemami59 aryaemami59 marked this pull request as ready for review October 26, 2024 22:11
Comment thread index.js Outdated
Comment thread package.json Outdated
Comment thread tsconfig.json Outdated
Comment thread types/index.d.ts Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread configs.js
Comment thread package.json Outdated

@scagood scagood left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Generally looks good to me 🙇‍♂️

Comment thread package.json Outdated
Comment thread types/configs.d.ts Outdated
@aryaemami59 aryaemami59 force-pushed the type-definitions branch 2 times, most recently from e05e75f to e603aa5 Compare November 13, 2024 11:30
@aryaemami59

Copy link
Copy Markdown
Author

I'm thinking about adding some type tests to instill some more confidence, let me know if that's something you would be interested in.

@scagood

scagood commented Nov 17, 2024

Copy link
Copy Markdown

That very much would be!

We have an issue in a different eslint-community repo to do exactly that (eslint-community/eslint-plugin-n#212). So I would (possibly) look to 'borrow' the prior art if you do end up adding some tests :)

Comment thread tests/types/configs.test-d.cts
Comment thread types/index.d.ts Outdated
@carlocorradini

Copy link
Copy Markdown

Any update? Thanks 🙌

@aryaemami59

Copy link
Copy Markdown
Author

Any update? Thanks 🙌

Sorry about that, I kind of got busy and neglected this, I did mention a while back that I think it's better if we do a full TS migration so the types remain accurate and stay in sync with the runtime code. That offer still stands, I would love to do that if you guys are okay with it.

@carlocorradini

Copy link
Copy Markdown

@aryaemami59 Awesome thanks 🙂‍↕️🥳

@aryaemami59 aryaemami59 force-pushed the type-definitions branch 2 times, most recently from 5ef2840 to 1e4b665 Compare March 2, 2025 13:23
@tomer-dev

Copy link
Copy Markdown

Looks great! Thank you 🚀

@aryaemami59 aryaemami59 force-pushed the type-definitions branch 2 times, most recently from f4d62f4 to 27f34f8 Compare April 11, 2025 13:40

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM! 🚀

+1 to shipping this now to resolve types issues for users of the plugin, then considering a full TS migration later.

Comment thread .github/workflows/ci.yml Outdated
Comment thread configs.js Outdated
Comment thread index.js

@michaelfaith michaelfaith left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🙌

@MichaelDeBoey MichaelDeBoey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but let's rebase onto latest main & revert all unrelated changes

@aryaemami59

Copy link
Copy Markdown
Author

I'll make the requested adjustmentts shortly.

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated

@MichaelDeBoey MichaelDeBoey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should make release dependent on these new workflows?
Otherwise a release could happen even if types aren't correct

CC/ @JoshuaKGoldberg

@MichaelDeBoey MichaelDeBoey changed the title feat: add missing type definitions feat: add type definitions Jan 13, 2026
Comment thread .github/workflows/ci.yml Outdated
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
@MichaelDeBoey MichaelDeBoey merged commit 10bd8ab into eslint-community:main Jan 14, 2026
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 4.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nnmrts

nnmrts commented Jan 18, 2026

Copy link
Copy Markdown

Sorry for pinging everybody in here, but shouldn't the type definitions also have a default export?

Before this PR, this line of code was fine:

import eslintComments from "@eslint-community/eslint-plugin-eslint-comments";

But now, with the type definitions, tools think this package has no default export. For example, I get this error from eslint-plugin-import-x: No default export found in imported module "@eslint-community/eslint-plugin-eslint-comments".

https://github.com/un-ts/eslint-plugin-import-x/blob/v4.16.1/docs/rules/default.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

missing TypeScript declaration file