Skip to content

Conversation

bmish
Copy link
Member

@bmish bmish commented Jun 21, 2025

Fixes #241.

This PR enables the checkJs TypeScript option for this plugin's codebase which requires us to add jsdoc type annotations anywhere types are missing. It also exports types for anyone who uses TypeScript in their eslint.config.js/ts.

The majority of the changes in this PR involve adding type annotations as well as adding type guards (checking node types).

Challenges:

  • Some types were tricky to get right
  • Some types required required refactoring to get right
  • Some node types needed to use import('eslint').Rule.Node when the node parent was provided by ESLint but others needed import('estree').Node when ESLint did not add the parent

It's likely this fixes a bunch of unknown bugs from not correctly checking node types or not checking for null/undefined variables.

This should significantly improve the developer experience from having autocomplete and type information available.

Follow-up PRs to consider:

  • This sets us up nicely for a future ESM/TypeScript conversion.

This took 10+ hours to do by hand. Cursor was helpful for autocomplete.

Related examples:

Follow-up fixes I needed to get this to work:

@bmish bmish force-pushed the jsdoc-typescript branch from d546e9c to 69e427a Compare June 21, 2025 22:30
@@ -43,21 +52,6 @@ module.exports = [
eqeqeq: "error",
"func-style": ["error", "declaration"],
"guard-for-in": "error",
"lines-around-comment": [
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a number of old rules I had to remove because they conflict with prettier.

eslint.config.js Outdated
@@ -137,14 +130,6 @@ module.exports = [
"spaced-comment": ["error", "always", { exceptions: ["-"] }],
strict: ["error", "global"],
"use-isnan": "error",
"valid-jsdoc": [
Copy link
Member Author

@bmish bmish Jun 21, 2025

Choose a reason for hiding this comment

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

This rule is deprecated and didn't appear to be compatible with the jsdoc TypeScript comments I added. We can replace with https://github.com/gajus/eslint-plugin-jsdoc later.

"functions": 100,
"branches": 100,
"branches": 90,
Copy link
Member Author

@bmish bmish Jun 21, 2025

Choose a reason for hiding this comment

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

Unfortunately, I had to reduce test coverage requirements because of the 100+ type guards I added. Some type guards could be tested, while most would necessitate /* istanbul ignore */ comments. We can add tests or ignore comments later as it would be very time-consuming to do so.

@bmish bmish changed the title feat: add jsdoc TypeScript types feat: add type-checking to all code with jsdoc TypeScript types Jun 21, 2025
@bmish bmish changed the title feat: add type-checking to all code with jsdoc TypeScript types feat: add type-checking with jsdoc TypeScript types Jun 21, 2025
@bmish bmish requested a review from platinumazure June 22, 2025 02:59
bmish added 2 commits June 22, 2025 20:48
* main:
  upgrade: Bump @eslint/js from 8.56.0 to 9.24.0 (qunitjs#568)
  upgrade: Bump eslint-plugin-markdown from 3.0.1 to 5.1.0 (qunitjs#542)
  chore: fix violations of eslint-plugin-eslint-plugin v6 (qunitjs#572)
  upgrade: Bump cross-spawn (qunitjs#551)
  upgrade: Bump braces from 3.0.2 to 3.0.3 (qunitjs#521)
  upgrade: Bump ws from 7.5.9 to 7.5.10 (qunitjs#524)
  upgrade: Bump eslint-plugin-eslint-plugin from 5.5.1 to 6.4.0 (qunitjs#537)
  upgrade: Bump serialize-javascript and mocha (qunitjs#550)
  upgrade: Bump @eslint/eslintrc from 2.1.4 to 3.3.1 (qunitjs#563)
  upgrade: Bump typescript from 5.3.3 to 5.8.3 (qunitjs#569)
  upgrade: Bump eslint-doc-generator from 1.6.2 to 2.1.2 (qunitjs#562)
@coveralls
Copy link

coveralls commented Jun 23, 2025

Pull Request Test Coverage Report for Build 15933563230

Details

  • 169 of 242 (69.83%) changed or added relevant lines in 26 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-7.9%) to 92.058%

Changes Missing Coverage Covered Lines Changed/Added Lines %
eslint-remote-tester.config.mjs 0 1 0.0%
lib/rules/literal-compare-order.js 7 8 87.5%
lib/rules/no-assert-logical-expression.js 6 7 85.71%
lib/rules/no-commented-tests.js 2 3 66.67%
lib/rules/no-conditional-assertions.js 1 2 50.0%
lib/rules/no-early-return.js 1 2 50.0%
lib/rules/no-setup-teardown.js 4 5 80.0%
lib/rules/no-throws-string.js 1 2 50.0%
lib/rules/assert-args.js 7 9 77.78%
lib/rules/no-compare-relation-boolean.js 2 4 50.0%
Files with Coverage Reduction New Missed Lines %
lib/utils.js 1 92.92%
Totals Coverage Status
Change from base Build 15890712979: -7.9%
Covered Lines: 1004
Relevant Lines: 1078

💛 - Coveralls

This was referenced Jun 23, 2025
@bmish
Copy link
Member Author

bmish commented Jun 26, 2025

I'm going to merge this now for a forthcoming minor release. I'm open to receiving and addressing feedback after the merge.

@bmish bmish changed the title feat: add type-checking with jsdoc TypeScript types feat: add type-checking with jsdoc TypeScript types and publish types Jun 27, 2025
@@ -22,7 +24,7 @@ module.exports = {
configs: {
recommended: {
plugins: ["qunit"],
rules: {
rules: /** @type {import('eslint').Linter.RulesRecord} */ ({
Copy link
Member Author

Choose a reason for hiding this comment

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

This was necessary so that consumers who use our flat config in their eslint.config.ts has the right RuleEntry type instead of just generic string.

@bmish bmish merged commit d543a4b into qunitjs:main Jun 27, 2025
6 checks passed
@Krinkle
Copy link
Member

Krinkle commented Jun 28, 2025

This is awesome, thank you @bmish!

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

Successfully merging this pull request may close these issues.

Add TypeScript type-checking to all rules
3 participants