fix(webdriverjs): fix default commonJs export

[straker]

This fixes the breaking change we accidentally released in 4.7.3 when we broke the default export of @axe-core/webdriverjs. Also added tests to ensure we don't make the same mistake again.

Closes #940

[stephenmathieson]

This PR touches a lot more than webdriverjs. May want to update the commit scope 🙂

[dbjorge]

It might be nice to also include tests of importing from typescript; we've seen at least one case where a CJS/ESM-related misconfiguration resulted in a package that could be used successfully from JS, but resulted in an empty typings file that broke importing from typescript. Maybe something like:

// This file isn't executed; we only care that typescript can successfully
// build/typecheck. It is a smoke test that our build process is producing a
// basically-reasonable index.d.ts.

// Detailed tests of individual types should instead be covered by unit tests.

import { AxeBuilder as NamedImportAxeBuilder } from '../dist/index.js';
import DefaultImportAxeBuilder from '../dist/index.js';
import type { WebDriver } from 'selenium-webdriver';

// See https://stackoverflow.com/a/55541672
type IsAny<T> = 0 extends (1 & T) ? true : false;

// If the imports don't have typings assigned, these will fail
// with "ts(2322): Type 'false' is not assignable to type 'true'."
(x: IsAny<typeof NamedImportAxeBuilder>): false => x;
(x: IsAny<typeof DefaultImportAxeBuilder>): false => x;

new NamedImportAxeBuilder({} as WebDriver)
  .withRules('color-contrast')
  .analyze();

new DefaultImportAxeBuilder({} as WebDriver)
  .withRules('color-contrast')
  .analyze();

[dbjorge]

It's unfortunate that this causes some warning spew in the build:

warning spew

/repos/axe-core-npm/packages/webdriverjs (webdriverjs-default-export*) » yarn build                               danbjorge@Dans-MacBook-Pro
yarn run v1.22.19
$ rimraf dist
$ tsup src/index.ts --dts --format esm,cjs
CLI Building entry: src/index.ts
CLI Using tsconfig: tsconfig.json
CLI tsup v6.7.0
CLI Target: esnext
ESM Build start
CJS Build start

 WARN  ▲ [WARNING] The CommonJS "module" variable is treated as a global variable in an ECMAScript module and may not work as expected [commonjs-variable-in-esm]

    src/index.ts:296:2:
      296 │   module.exports = AxeBuilder
          ╵   ~~~~~~

  This file is considered to be an ECMAScript module because of the "export" keyword here:

    src/index.ts:301:0:
      301 │ export { AxeBuilder };
          ╵ ~~~~~~




 WARN  ▲ [WARNING] The CommonJS "module" variable is treated as a global variable in an ECMAScript module and may not work as expected [commonjs-variable-in-esm]

    src/index.ts:296:2:
      296 │   module.exports = AxeBuilder
          ╵   ~~~~~~

  This file is considered to be an ECMAScript module because of the "export" keyword here:

    src/index.ts:301:0:
      301 │ export { AxeBuilder };
          ╵ ~~~~~~



ESM dist/index.mjs 14.24 KB
ESM ⚡️ Build success in 39ms
CJS dist/index.js 15.97 KB
CJS ⚡️ Build success in 39ms
DTS Build start
^[[A^[[ADTS ⚡️ Build success in 809ms
DTS dist/index.d.ts 2.52 KB
✨  Done in 1.27s.

This is a reasonable warning, but one that we reasonably want to suppress here, I think. I didn't see a good way of doing that - it comes from esbuild (not eslint), which doesn't support a suppress-next-line type comment. esbuild does have a --log-override option to quiet it, but only for the whole build process, not for a few lines, and tsup doesn't support forwarding it anyway (they tried but encountered some issues getting it to work and abandoned it :( )

I don't think it's worth more time trying to suppress the warning, just noting it here so noone repeats the same research.

[straker]

I'm going to split the TS type check and the support for default/.default/named export into their own prs, though I will add the default/.default/named export into webdriverjs to ensure we don't break 4.7.3 with 4.7.4.

Resolved

[dbjorge]

LGTM, fine with leaving for a separate PR the ts tests + updating the rest of the packages to use the esbuild plugin we discussed