Skip to content

Commit

Permalink
Add stricter TypeScript type checking
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mbland committed Jan 16, 2024
1 parent 532bf70 commit e377922
Show file tree
Hide file tree
Showing 16 changed files with 771 additions and 268 deletions.
2 changes: 1 addition & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
],
"extends": [
"eslint:recommended",
"plugin:jsdoc/recommended"
"plugin:jsdoc/recommended-typescript-flavor-error"
],
"overrides": [
{
Expand Down
1 change: 0 additions & 1 deletion ci/vitest.config.browser.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import { defineConfig, mergeConfig } from 'vitest/config'
import baseConfig from './vitest.config.js'

Expand Down
1 change: 0 additions & 1 deletion ci/vitest.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import { defineConfig, mergeConfig } from 'vitest/config'
import viteConfig from '../vite.config.js'

Expand Down
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import BrowserPageOpener from './lib/browser.js'
import JsdomPageOpener from './lib/jsdom.js'
import { OpenedPage } from './lib/types.js'
import {OpenedPage} from './lib/types.js' // eslint-disable-line no-unused-vars

/**
* Enables tests to open an application's own page URLs both in the browser and
Expand Down
16 changes: 0 additions & 16 deletions jsconfig.json

This file was deleted.

4 changes: 4 additions & 0 deletions jsdoc.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
{
"plugins": [
"plugins/markdown",
"jsdoc-plugin-typescript",
"node_modules/jsdoc-plugin-intersection"
],
"recurseDepth": 10,
"source": {
"includePattern": ".+\\.js$",
"exclude": ["node_modules", "test"]
},
"typescript": {
"moduleRoot": "."
},
"opts": {
"destination": "jsdoc",
"recurse": true,
Expand Down
7 changes: 5 additions & 2 deletions lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
*/

import libCoverage from 'istanbul-lib-coverage'
import { OpenedPage } from './types.js'
import {OpenedPage} from './types.js' // eslint-disable-line no-unused-vars

/**
* @typedef {import("istanbul-lib-coverage").CoverageMap} CoverageMap
*/
/**
* Type for accessing the Istanbul coverage object in Window
* @typedef {Window & Object.<string, libCoverage.CoverageMap>} CovWindow
* @typedef {Window & Object.<string, CoverageMap>} CovWindow
*/

/**
Expand Down
6 changes: 3 additions & 3 deletions lib/jsdom.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

import { OpenedPage } from './types.js'
import {OpenedPage} from './types.js' // eslint-disable-line no-unused-vars

/**
* @typedef {object} JSDOM - simulated jsdom.JSDOM
Expand Down Expand Up @@ -98,7 +98,7 @@ export default class JsdomPageOpener {
* @param {Window} window - the jsdom window object
* @param {Document} document - the jsdom window.document object
* @returns {Promise<void>} - resolves after importing all ECMAScript modules
* @throws if importing any ECMAScript modules fails
* @throws {Error} if importing any ECMAScript modules fails
*/
#importModules(window, document) {
return new Promise((resolve, reject) => {
Expand Down Expand Up @@ -180,7 +180,7 @@ export default class JsdomPageOpener {
* Only works with the `src` attribute; it will not execute inline code.
* @param {Document} doc - the jsdom window.document object
* @returns {Promise<void[]>} - resolves after importing all modules in doc
* @throws if any module import fails
* @throws {Error} if any module import fails
*/
function importModules(doc) {
/** @type {HTMLScriptElement[]} */
Expand Down
32 changes: 16 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@
"test:ci:jsdom": "vitest run -c ci/vitest.config.js",
"test:ci:browser": "vitest run -c ci/vitest.config.browser.js",
"jsdoc": "jsdoc-cli-wrapper -c jsdoc.json .",
"typecheck": "npx -p typescript tsc -p jsconfig.json --noEmit --pretty",
"prepack": "npx -p typescript tsc ./index.js --allowJs --declaration --declarationMap --emitDeclarationOnly --outDir types"
"typecheck": "npx tsc",
"prepack": "npx rimraf types && npx tsc ./index.js --allowJs --declaration --declarationMap --emitDeclarationOnly --outDir types"
},
"files": [
"index.js",
"lib/*",
"types/*"
"lib/**",
"types/**"
],
"keywords": [
"testing",
Expand All @@ -28,33 +27,34 @@
"license": "MPL-2.0",
"type": "module",
"engines": {
"node": ">= 18.0.0"
"node": ">=18.0.0"
},
"homepage": "https://github.com/mbland/test-page-opener",
"repository": "https://github.com/mbland/test-page-opener",
"bugs": "https://github.com/mbland/test-page-opener/issues",
"devDependencies": {
"@stylistic/eslint-plugin-js": "^1.5.3",
"@types/chai": "^4.3.11",
"@types/istanbul-lib-coverage": "^2.0.6",
"@types/jsdom": "^21.1.6",
"@vitest/browser": "^1.1.3",
"@vitest/coverage-istanbul": "^1.1.3",
"@vitest/coverage-v8": "^1.1.3",
"@vitest/ui": "^1.1.3",
"@vitest/browser": "^1.2.0",
"@vitest/coverage-istanbul": "^1.2.0",
"@vitest/coverage-v8": "^1.2.0",
"@vitest/ui": "^1.2.0",
"eslint": "^8.56.0",
"eslint-plugin-jsdoc": "^46.10.1",
"eslint-plugin-vitest": "^0.3.20",
"jsdoc": "^4.0.2",
"jsdoc-cli-wrapper": "^1.0.5",
"jsdoc-cli-wrapper": "^1.0.6",
"jsdoc-plugin-intersection": "^1.0.4",
"jsdoc-plugin-typescript": "^2.2.1",
"jsdom": "^23.2.0",
"rimraf": "^5.0.5",
"typescript": "^5.3.3",
"vite": "^5.0.11",
"vitest": "^1.1.3",
"webdriverio": "^8.27.0"
"vitest": "^1.2.0",
"webdriverio": "^8.27.2"
},
"dependencies": {
"@types/istanbul-lib-coverage": "^2.0.6",
"@types/jsdom": "^21.1.6",
"istanbul-lib-coverage": "^3.2.2"
}
}
Loading

0 comments on commit e377922

Please sign in to comment.