Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no-undefined-types: false positives for built-in interface types in typescript mode #888

Closed
tschaub opened this issue May 24, 2022 · 7 comments · Fixed by #1098
Closed

Comments

@tschaub
Copy link
Contributor

tschaub commented May 24, 2022

Expected behavior

No error.

Actual behavior

The type 'Float32ArrayConstructor' is undefined.
The type 'Uint32ArrayConstructor' is undefined.

ESLint Config

{
  "env": {
    "es6": true
  },
  "parserOptions": {
    "ecmaVersion": "latest",
    "sourceType": "module"
  },
  "plugins": ["jsdoc"],
  "rules": {
    "jsdoc/no-undefined-types": "error"
  },
  "settings": {
    "jsdoc": {
      "mode": "typescript"
    }
  }
}
const ELEMENT_ARRAY_BUFFER = 0x8893;
const ARRAY_BUFFER = 0x8892;

/**
 * Returns a typed array constructor based on the given buffer type
 * @param {number} type Buffer type, either ARRAY_BUFFER or ELEMENT_ARRAY_BUFFER.
 * @return {Float32ArrayConstructor|Uint32ArrayConstructor} The typed array class to use for this buffer.
 */
export function getArrayClassForType(type) {
  switch (type) {
    case ARRAY_BUFFER:
      return Float32Array;
    case ELEMENT_ARRAY_BUFFER:
      return Uint32Array;
    default:
      return Float32Array;
  }
}

Environment

  • Node version: v17.5.0
  • ESLint version v8.0.0
  • eslint-plugin-jsdoc version: 39.3.2

Details

When using "mode": "typescript" it looks like TypeScript's built-in interface types are not recognized as defined types (e.g. the es2017 typed array types).

Let me know if you'd accept a pull request to add more extra/known types to the list used by the rule.

@brettz9
Copy link
Collaborator

brettz9 commented May 24, 2022

Sure. For a PR, I think it'd be ideal to be systematic in covering currently supported items as a whole rather than piecemeal, though adding comments as to the source file of additions.

@brettz9
Copy link
Collaborator

brettz9 commented Oct 23, 2022

Since this rule is meant to warn about types that are not available, I think we'd really need a way (for TS users) to check the tsconfig.json file so as to know if es5 globals, etc. should be loaded.

@AustinGil
Copy link

Looking for existing issues and I think this is the place, but is this why JSDoc comment complains that things like 'Record', 'Pick', 'T', 'NodeListOf', or 'SubmitEvent' are not defined?

@brettz9
Copy link
Collaborator

brettz9 commented Dec 8, 2022

Looking for existing issues and I think this is the place, but is this why JSDoc comment complains that things like 'Record', 'Pick', 'T', 'NodeListOf', or 'SubmitEvent' are not defined?

Yes.

@jaydenseric
Copy link
Contributor

As discussed here, the config plugin:jsdoc/recommended-typescript-error should disable the jsdoc/no-undefined-types rule because TypeScript itself is responsible for reporting errors about invalid JSDoc types.

When in TypeScript mode, the rule jsdoc/no-undefined-types doesn't just have false errors about global types; if you do {@link foo} within a class constructor parameter JSDoc description, where foo references a property/method of the same class, TypeScript is smart enough to know what is linked, but the rule jsdoc/no-undefined-types reports an error unless you change it to {@link ClassName.foo}.

@tschaub
Copy link
Contributor Author

tschaub commented May 12, 2023

@jaydenseric - That sounds reasonable to me.

We no longer use the jsdoc/no-undefined-types rule due to this issue. To be honest, I'm uncertain if any problems (undetected undefined things) are sneaking through, or if this is the right solution.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 26, 2023
…us#888

BREAKING CHANGE:

This should only impact users of typescript configs. TS should itself handle
checking for undefined types, so the (imperfect) rule has been disabled for
such users.
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 28, 2023
…us#888

BREAKING CHANGE:

This should only impact users of typescript configs. TS should itself handle
checking for undefined types, so the (imperfect) rule has been disabled for
such users.
brettz9 added a commit that referenced this issue May 28, 2023
BREAKING CHANGE:

This should only impact users of typescript configs. TS should itself handle
checking for undefined types, so the (imperfect) rule has been disabled for
such users.
@github-actions
Copy link

🎉 This issue has been resolved in version 45.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Jun 6, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc) | devDependencies | major | [`40.3.0` -> `46.2.4`](https://renovatebot.com/diffs/npm/eslint-plugin-jsdoc/40.3.0/46.2.4) |

---

### Release Notes

<details>
<summary>gajus/eslint-plugin-jsdoc</summary>

### [`v46.2.4`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.2.4)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.2.3...v46.2.4)

##### Bug Fixes

-   **`imports-as-dependencies`:** allow relative paths ([7469e59](gajus/eslint-plugin-jsdoc@7469e59))

### [`v46.2.3`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.2.3)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.2.2...v46.2.3)

##### Bug Fixes

-   **`imports-as-dependencies`:** catch `typings` as possible publishing source ([e3b0d0c](gajus/eslint-plugin-jsdoc@e3b0d0c))

### [`v46.2.2`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.2.2)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.2.1...v46.2.2)

##### Bug Fixes

-   **`imports-as-dependencies`:** don't report TypeScript proper ([75b6b8c](gajus/eslint-plugin-jsdoc@75b6b8c))

### [`v46.2.1`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.2.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.2.0...v46.2.1)

##### Bug Fixes

-   **`imports-as-dependencies`:** check for `types` in `package.json` and if not present, check `[@types](https://github.com/types)`; fixes [#&#8203;1107](gajus/eslint-plugin-jsdoc#1107) ([785fb26](gajus/eslint-plugin-jsdoc@785fb26))

### [`v46.2.0`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.2.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.1.0...v46.2.0)

##### Features

-   **`imports-as-dependencies`:** add new rule to detect missing dependencies for import statements; fixes [#&#8203;896](gajus/eslint-plugin-jsdoc#896) ([d7ec6e0](gajus/eslint-plugin-jsdoc@d7ec6e0))

### [`v46.1.0`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.1.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v46.0.0...v46.1.0)

##### Features

-   **`require-returns`:** per-context `forceRequireReturn`; fixes [#&#8203;757](gajus/eslint-plugin-jsdoc#757) ([31b3a24](gajus/eslint-plugin-jsdoc@31b3a24))

### [`v46.0.0`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v46.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v45.0.0...v46.0.0)

##### Features

-   **`no-defaults`:** make no-defaults on by default ([034ade1](gajus/eslint-plugin-jsdoc@034ade1))

##### BREAKING CHANGES

-   **`no-defaults`:** Removes default values. Disable the rule if removal not desired.

Also:

-   docs: adds info on TS recommended rules

### [`v45.0.0`](https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v45.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.7...v45.0.0)

##### Features

-   add recommended-typescript-flavor configs ([6042b77](gajus/eslint-plugin-jsdoc@6042b77))
-   for typescript configs, disable `no-undefined-types`; fixes [#&#8203;888](gajus/eslint-plugin-jsdoc#888) ([ec41b90](gajus/eslint-plugin-jsdoc@ec41b90))

##### BREAKING CHANGES

-   This should only impact users of typescript configs. TS should itself handle
    checking for undefined types, so the (imperfect) rule has been disabled for
    such users.

### [`v44.2.7`](gajus/eslint-plugin-jsdoc@v44.2.6...v44.2.7)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.6...v44.2.7)

### [`v44.2.6`](gajus/eslint-plugin-jsdoc@v44.2.5...v44.2.6)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.5...v44.2.6)

### [`v44.2.5`](gajus/eslint-plugin-jsdoc@v44.2.4...v44.2.5)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.4...v44.2.5)

### [`v44.2.4`](gajus/eslint-plugin-jsdoc@v44.2.3...v44.2.4)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.3...v44.2.4)

### [`v44.2.3`](gajus/eslint-plugin-jsdoc@v44.2.2...v44.2.3)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.2...v44.2.3)

### [`v44.2.2`](gajus/eslint-plugin-jsdoc@v44.2.1...v44.2.2)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.1...v44.2.2)

### [`v44.2.1`](gajus/eslint-plugin-jsdoc@v44.2.0...v44.2.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.2.0...v44.2.1)

### [`v44.2.0`](gajus/eslint-plugin-jsdoc@v44.1.0...v44.2.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.1.0...v44.2.0)

### [`v44.1.0`](gajus/eslint-plugin-jsdoc@v44.0.2...v44.1.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.0.2...v44.1.0)

### [`v44.0.2`](gajus/eslint-plugin-jsdoc@v44.0.1...v44.0.2)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.0.1...v44.0.2)

### [`v44.0.1`](gajus/eslint-plugin-jsdoc@v44.0.0...v44.0.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v44.0.0...v44.0.1)

### [`v44.0.0`](gajus/eslint-plugin-jsdoc@v43.2.0...v44.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.2.0...v44.0.0)

### [`v43.2.0`](gajus/eslint-plugin-jsdoc@v43.1.1...v43.2.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.1.1...v43.2.0)

### [`v43.1.1`](gajus/eslint-plugin-jsdoc@v43.1.0...v43.1.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.1.0...v43.1.1)

### [`v43.1.0`](gajus/eslint-plugin-jsdoc@v43.0.9...v43.1.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.9...v43.1.0)

### [`v43.0.9`](gajus/eslint-plugin-jsdoc@v43.0.8...v43.0.9)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.8...v43.0.9)

### [`v43.0.8`](gajus/eslint-plugin-jsdoc@v43.0.7...v43.0.8)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.7...v43.0.8)

### [`v43.0.7`](gajus/eslint-plugin-jsdoc@v43.0.6...v43.0.7)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.6...v43.0.7)

### [`v43.0.6`](gajus/eslint-plugin-jsdoc@v43.0.5...v43.0.6)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.5...v43.0.6)

### [`v43.0.5`](gajus/eslint-plugin-jsdoc@v43.0.4...v43.0.5)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.4...v43.0.5)

### [`v43.0.4`](gajus/eslint-plugin-jsdoc@v43.0.3...v43.0.4)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.3...v43.0.4)

### [`v43.0.3`](gajus/eslint-plugin-jsdoc@v43.0.2...v43.0.3)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.2...v43.0.3)

### [`v43.0.2`](gajus/eslint-plugin-jsdoc@v43.0.1...v43.0.2)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.1...v43.0.2)

### [`v43.0.1`](gajus/eslint-plugin-jsdoc@v43.0.0...v43.0.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v43.0.0...v43.0.1)

### [`v43.0.0`](gajus/eslint-plugin-jsdoc@v42.0.0...v43.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v42.0.0...v43.0.0)

### [`v42.0.0`](gajus/eslint-plugin-jsdoc@v41.1.2...v42.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v41.1.2...v42.0.0)

### [`v41.1.2`](gajus/eslint-plugin-jsdoc@v41.1.1...v41.1.2)

[Compare Source](gajus/eslint-plugin-jsdoc@v41.1.1...v41.1.2)

### [`v41.1.1`](gajus/eslint-plugin-jsdoc@v41.1.0...v41.1.1)

[Compare Source](gajus/eslint-plugin-jsdoc@v41.1.0...v41.1.1)

### [`v41.1.0`](gajus/eslint-plugin-jsdoc@v41.0.0...v41.1.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v41.0.0...v41.1.0)

### [`v41.0.0`](gajus/eslint-plugin-jsdoc@v40.3.0...v41.0.0)

[Compare Source](gajus/eslint-plugin-jsdoc@v40.3.0...v41.0.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTEuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMS4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1913
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
mbland added a commit to mbland/rollup-plugin-handlebars-precompiler that referenced this issue Jan 10, 2024
The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/{chai,node}, jsdoc, and typescript as devDependencies.

- Added JSDoc-based @typedefs, including the standalone lib/types.js
  based on: "Stack Overflow: How to 'import' a typedef from one file to
  another in JSDoc using Node.js?"

  - https://stackoverflow.com/a/76872194

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor-error"]
  > }
  > ```
  >
  > - https://github.com/gajus/eslint-plugin-jsdoc#eslintrc

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- At the same time, extending "recommended-typescript-flavor-error"
  required adding the `// eslint-disable-next-line no-unused-vars`
  directive before each set of imports from lib/types.js.

- Added test/vitest.d.ts so TypeScript could find the custom toStartWith
  and toEndWith expect extension matchers.

- Added `pnpm typecheck && pnpm jsdoc` to `pnpm test:ci`.
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/test-page-opener that referenced this issue Jan 16, 2024
Most of this message is copied from:

- mbland/jsdoc-cli-wrapper#21
  mbland/jsdoc-cli-wrapper@e0d287d
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

After doing more research, it appears tsconfig.json is more broadly
supported by editors and IDEs than jsconfig.json. For example, IntelliJ
IDEA/WebStorm won't recognize it unless added to Settings > Editor >
File Types > TypeScript.

Related changes include:

- Fixed the problems with vitest.config.js and ci/vitest.config.js such
  that the `// @ts-nocheck` directive is no longer necessary. Used
  `...(configDefaults.coverage.exclude || [])` to avoid type errors due
  to configDefaults.coverage.exclude potentially being undefined.

- Moved a bunch of compiler options from the `pnpm typecheck` script
  into tsconfig.json.

- Added the `rimraf` npm to make sure `pnpm prepack` generates a new
  `types/` directory without stale content.

- Added @types/{jsdom,istanbul-lib-coverage} as production dependencies.

- Bumped vitest to 1.2.0.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor-error"]
  > }
  > ```
  >
  > - https://github.com/gajus/eslint-plugin-jsdoc#eslintrc

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- At the same time, extending "recommended-typescript-flavor-error"
  required adding the `// eslint-disable-next-line no-unused-vars`
  directive before each set of imports from lib/types.js. (Or adding
  `// eslint-disable-line no-unused-vars` on the same line if possible.)

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Finally, it seems IntelliJ IDEA's JSDoc type checking is stronger than
  TypeScript and VSCode. Fixed a few JSDoc type parameters to eliminate
  warnings in IntelliJ as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@brettz9 @tschaub @jaydenseric @AustinGil and others