Merge pull request #23 from mbland/doc-update-typescript-tweaks

Bump to v1.0.5, update README, strict type checks

.eslintrc

@@ -1,7 +1,7 @@
 {
-  "env" : {
+  "env": {
     "node": true,
-    "es2023" : true
+    "es2023": true
   },
   "parserOptions": {
     "ecmaVersion": "latest",
@@ -25,7 +25,7 @@
       ]
     }
   ],
-  "rules" : {
+  "rules": {
     "@stylistic/js/comma-dangle": [
       "error", "never"
     ],
@@ -50,5 +50,12 @@
     "no-console": [
       "error", { "allow": [ "warn", "error" ]}
     ]
+  },
+  "settings": {
+    "jsdoc": {
+      "preferredTypes": {
+        "Object": "object"
+      }
+    }
   }
 }

README.md

@@ -43,6 +43,39 @@ describe('TestPageOpener', () => {
 })
 ```
 
+### Using with a bundler (e.g., with Rollup, Vite, and Vitest)
+
+If your project uses any bundler plugins that perform source transforms, you
+_may_ need to configure your project to include `test-page-loader` in the test
+bundle. Specifically, if it transforms files without a `.js` extension into importable JavaScript, `test-page-opener` may fail with an error resembling:
+
+```text
+Caused by: TypeError: Unknown file extension ".hbs" for
+/.../mbland/tomcat-servlet-testing-example/strcalc/src/main/frontend/components/calculator.hbs
+————————————————————————————————————————————————————————
+Serialized Error: { code: 'ERR_UNKNOWN_FILE_EXTENSION' }
+————————————————————————————————————————————————————————
+```
+
+For example, using [Vite][] and [Vitest][], which use [Rollup][] under the hood,
+you will need to add this `server:` setting to the `test` config object:
+
+```js
+test: {
+  server: {
+    deps: {
+      // Without this, jsdom tests will fail to import '.hbs' files
+      // transformed by rollup-plugin-handlebars-precompiler.
+      inline: ['test-page-opener']
+    }
+  }
+}
+```
+
+For a concrete example with more details, see:
+
+- <https://github.com/mbland/tomcat-servlet-testing-example/pull/83>
+
 ### Reporting code coverage
 
 `TestPageOpener` makes it possible to collect code coverage from opened browser
@@ -229,11 +262,12 @@ level explanation.
 [coveralls-tpo]: https://coveralls.io/github/mbland/test-page-opener?branch=main
 [npm-tpo]: https://www.npmjs.com/package/test-page-opener
 [pnpm]: https://pnpm.io/
+[Vite]: https://vitejs.dev/
+[Vitest]: https://vitest.dev/
+[Rollup]: https://rollupjs.org/
 [DOMContentLoaded]: https://developer.mozilla.org/docs/Web/API/Document/DOMContentLoaded_event
 [window.load]: https://developer.mozilla.org/docs/Web/API/Window/load_event
 [DOM]: https://developer.mozilla.org/docs/Web/API/Document_Object_Model
-[Vite]: https://vitejs.dev/
-[Vitest]: https://vitest.dev/
 [ECMAScript Modules]: https://nodejs.org/docs/latest-v18.x/api/esm.html
 [ESM resolution and loading algorithm]: https://nodejs.org/docs/latest-v18.x/api/esm.html#resolution-and-loading-algorithm
 [jsdom-2475]: https://github.com/jsdom/jsdom/issues/2475

ci/vitest.config.browser.js

@@ -6,6 +6,7 @@ export default mergeConfig(baseConfig, defineConfig({
   test: {
     outputFile: 'TESTS-TestSuites-browser.xml',
     coverage: {
+      provider: 'istanbul',
       reportsDirectory: 'coverage-browser'
     },
     browser: {

ci/vitest.config.js

@@ -8,7 +8,6 @@ export default mergeConfig(viteConfig, defineConfig({
     reporters: [ 'junit', 'default' ],
     coverage: {
       enabled: true,
-      provider: 'istanbul',
       reporter: [ 'text', 'lcovonly' ],
       reportsDirectory: 'coverage-jsdom'
     }

index.js

@@ -102,6 +102,7 @@ export default class TestPageOpener {
    * @param {string} pagePath - path to the HTML file relative to the basePath
    *   specified during `TestPageOpener.create()`
    * @returns {Promise<OpenedPage>} - object representing the opened page
+   * @throws {Error} if pagePath is malformed or opening page failed
    */
   async open(pagePath) {
     if (pagePath.startsWith('/')) {

jsconfig.json

@@ -1,10 +1,16 @@
 {
   "compilerOptions": {
     "checkJs": true,
-    "module": "nodenext",
-    "strict": true,
-    "strictNullChecks": false,
-    "noImplicitAny": false
+    "lib": [
+      "ES2022"
+    ],
+    "module": "node16",
+    "target": "es2020",
+    "strict": true
   },
-  "exclude": ["node_modules"]
+  "exclude": [
+    "node_modules/**",
+    "coverage*/**",
+    "jsdoc/**"
+  ]
 }

jsdoc.json

@@ -1,11 +1,12 @@
 {
   "plugins": [
-    "plugins/markdown"
+    "plugins/markdown",
+    "node_modules/jsdoc-plugin-intersection"
   ],
   "recurseDepth": 10,
   "source": {
     "includePattern": ".+\\.js$",
-    "exclude": ["node_modules"]
+    "exclude": ["node_modules", "test"]
   },
   "opts": {
     "destination": "jsdoc",

lib/browser.js

@@ -8,6 +8,11 @@
 import libCoverage from 'istanbul-lib-coverage'
 import { OpenedPage } from './types.js'
 
+/**
+ * Type for accessing the Istanbul coverage object in Window
+ * @typedef {Window & Object.<string, libCoverage.CoverageMap>} CovWindow
+ */
+
 /**
  * Returns the window and document from a browser-opened HTML file.
  *
@@ -20,6 +25,7 @@ import { OpenedPage } from './types.js'
 export default class BrowserPageOpener {
   #window
   #coverageKey
+  #coverageMap
 
   /**
    * @param {Window} window - the global (browser) window object
@@ -34,19 +40,26 @@ export default class BrowserPageOpener {
     // coverage. There's no harm in this, and it avoids a coverage gap for a
     // condition that, by definition, would never execute when collecting
     // coverage. We could use a directive to ignore that gap, but why bother.
-    window[covKey] = libCoverage.createCoverageMap(window[covKey])
+    const covWindow = /** @type {CovWindow} */ (window)
+    this.#coverageMap = libCoverage.createCoverageMap(covWindow[covKey])
+    covWindow[covKey] = this.#coverageMap
   }
 
   /**
    * Opens another page within a web browser.
    * @param {string} basePath - base path of the application under test
    * @param {string} pagePath - path to the HTML file relative to basePath
    * @returns {Promise<OpenedPage>} - object representing the opened page
+   * @throws {Error} if opening page failed
    */
   async open(basePath, pagePath) {
-    const w = this.#window.open(`${basePath}${pagePath}`)
+    const fullPath = `${basePath}${pagePath}`
+    const w = this.#window.open(fullPath)
+    if (w === null) throw new Error(`failed to open: ${fullPath}`)
+
     const close = () => {
-      this.#window[this.#coverageKey].merge(w[this.#coverageKey])
+      const testWindow = /** @type {CovWindow} */ (w)
+      this.#coverageMap.merge(testWindow[this.#coverageKey])
       w.close()
     }
 

lib/jsdom.js

@@ -7,6 +7,11 @@
 
 import { OpenedPage } from './types.js'
 
+/**
+ * @typedef {object} JSDOM - simulated jsdom.JSDOM
+ * @property {Function} fromFile - simulated JSDOM.fromFile
+ */
+
 /**
  * Returns window and document objects from a jsdom-parsed HTML file.
  *
@@ -61,7 +66,7 @@ export default class JsdomPageOpener {
   /**
    * Creates a JsdomPageOpener from a dynamically imported jsdom module
    * @param {object} jsdom - dynamically imported jsdom module
-   * @param {object} jsdom.JSDOM - JSDOM class from the jsdom module
+   * @param {JSDOM} jsdom.JSDOM - JSDOM class from the jsdom module
    */
   constructor({ JSDOM }) {
     this.#JSDOM = JSDOM
@@ -72,6 +77,7 @@ export default class JsdomPageOpener {
    * @param {string} _ - ignored
    * @param {string} pagePath - path to the HTML file to load
    * @returns {Promise<OpenedPage>} - object representing the opened page
+   * @throws {Error} if opening page failed
    */
   async open(_, pagePath) {
     const { window } = await this.#JSDOM.fromFile(
@@ -125,35 +131,40 @@ export default class JsdomPageOpener {
         // register closures over window and document, or specific document
         // elements. That would ensure they remain defined even after we remove
         // window and document from globalThis.
-        //
+        const { window: origWindow, document: origDocument } = globalThis
+
+        /** @param {Function} done - called after restoring original globals */
+        const resetGlobals = done => {
+          globalThis.document = origDocument
+          globalThis.window = origWindow
+          done()
+        }
+
         // @ts-expect-error
         globalThis.window = window
         globalThis.document = document
-        const Event = globalThis.window.Event
 
         try { await importModules(document) }
-        catch (err) { reject(err) }
+        catch (err) { return resetGlobals(() => {reject(err)}) }
 
         // Manually firing DOMContentLoaded again after loading modules
         // approximates the requirement that modules execute before
         // DOMContentLoaded. This means that the modules can register
         // DOMContentLoaded event listeners and have them fire here.
         //
         // We eventually fire the 'load' event again too for the same reason.
-        document.dispatchEvent(new Event(
-          'DOMContentLoaded', {bubbles: true, cancelable: false}
-        ))
+        const Event = globalThis.window.Event
+        document.dispatchEvent(
+          new Event('DOMContentLoaded', {bubbles: true, cancelable: false})
+        )
 
         // Register a 'load' listener that deletes the global window and
         // document variables. Because it's registered after any
         // DOMContentLoaded listeners have fired, it should execute after any
         // other 'load' listeners registered by any module code.
-        const resetGlobals = () => {
-          delete globalThis.document
-          delete globalThis.window
-          resolve()
-        }
-        window.addEventListener('load', resetGlobals, {once: true})
+        window.addEventListener(
+          'load', () => {resetGlobals(resolve)}, {once: true}
+        )
         window.dispatchEvent(
           new Event('load', {bubbles: false, cancelable: false})
         )

package.json

@@ -1,14 +1,17 @@
 {
   "name": "test-page-opener",
-  "version": "1.0.4",
+  "version": "1.0.5",
   "description": "Enables an application's tests to open its own page URLs both in the browser and in Node.js using jsdom",
   "main": "index.js",
   "types": "types/index.d.ts",
   "scripts": {
     "lint": "eslint --color --max-warnings 0 .",
     "test": "vitest",
-    "test:ci": "eslint --color --max-warnings 0 . && vitest run -c ci/vitest.config.js && vitest run -c ci/vitest.config.browser.js",
+    "test:ci": "pnpm lint && pnpm typecheck && pnpm test:ci:jsdom && pnpm test:ci:browser",
+    "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"
   },
   "files": [
@@ -32,6 +35,9 @@
   "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",
@@ -40,6 +46,7 @@
     "eslint-plugin-jsdoc": "^46.10.1",
     "eslint-plugin-vitest": "^0.3.20",
     "jsdoc-cli-wrapper": "^1.0.4",
+    "jsdoc-plugin-intersection": "^1.0.4",
     "jsdom": "^23.1.0",
     "typescript": "^5.3.3",
     "vite": "^5.0.11",