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.

Sign up for GitHub

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

fix(webdriverjs): fix default commonJs export #927

Merged
merged 5 commits into from
Nov 21, 2023
Merged

fix(webdriverjs): fix default commonJs export #927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5425097
fix(webdriverjs): fix default commonJs export
straker Nov 16, 2023
86974ed
fix react building on test
straker Nov 16, 2023
3891835
fix now?
straker Nov 17, 2023
595a3cf
revert example update
straker Nov 17, 2023
bdb3975
support 4.7.3
straker Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
working-directory: packages/puppeteer
- run: npm run build --workspace=packages/puppeteer
- run: npm run coverage --workspace=packages/puppeteer
- run: npm run test:esm --workspace=packages/puppeteer
- run: npm run test:export --workspace=packages/puppeteer

cli:
strategy:
Expand Down Expand Up @@ -83,7 +83,7 @@ jobs:
working-directory: packages/webdriverjs
- run: npm run build --workspace=packages/webdriverjs
- run: npm run coverage --workspace=packages/webdriverjs
- run: npm run test:esm --workspace=packages/webdriverjs
- run: npm run test:export --workspace=packages/webdriverjs

webdriverio:
strategy:
Expand All @@ -105,7 +105,7 @@ jobs:
working-directory: packages/webdriverio
- run: npm run build --workspace=packages/webdriverio
- run: npm run coverage --workspace=packages/webdriverio
- run: npm run test:esm --workspace=packages/webdriverio
- run: npm run test:export --workspace=packages/webdriverio

reporter_earl:
strategy:
Expand All @@ -123,7 +123,7 @@ jobs:
- run: npm ci
- run: npm run build --workspace=packages/reporter-earl
- run: npm run test --workspace=packages/reporter-earl
- run: npm run test:esm --workspace=packages/reporter-earl
- run: npm run test:export --workspace=packages/reporter-earl

react:
strategy:
Expand All @@ -139,9 +139,12 @@ jobs:
node-version: ${{ matrix.node }}
cache: 'npm'
- run: npm ci
- run: npm run build --workspace=packages/react
- run: npm run test --workspace=packages/react
- run: npm run test:esm --workspace=packages/react
# the tests builds the project using tsc and relies on `cache.ts` to be
# built and be it's own file. however we don't want that for the export
# test so we need to rebuild using tsup
- run: npm run build --workspace=packages/react
- run: npm run test:export --workspace=packages/react

playwright:
strategy:
Expand All @@ -160,7 +163,7 @@ jobs:
- run: npx playwright install --with-deps
- run: npm run build --workspace=packages/playwright
- run: npm run coverage --workspace=packages/playwright
- run: npm run test:esm --workspace=packages/playwright
- run: npm run test:export --workspace=packages/playwright

wdio_globals_test:
strategy:
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
"prebuild": "rimraf dist",
"build": "tsup src/index.ts --dts --format esm,cjs",
"test": "mocha --timeout 60000 -r ts-node/register 'test/**.spec.ts'",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node test/esmTest.mjs",
"test:commonjs": "node test/commonjsTest.js",
"coverage": "nyc npm run test",
"prepare": "npx playwright install && npm run build"
},
Expand Down
11 changes: 11 additions & 0 deletions packages/playwright/test/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// ensure backwards compatibility of commonJs format
const defaultExport = require('../dist/index.js').default;
const { AxeBuilder } = require('../dist/index.js');
const assert = require('assert');

assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxeBuilder === 'function', 'named export is not a function');
assert(
defaultExport === AxeBuilder,
'default and named export are not the same'
);
7 changes: 5 additions & 2 deletions packages/playwright/test/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// ensure compatibility of ESM format
import defaultExport from '../dist/index.mjs';
import { AxeBuilder } from '../dist/index.mjs';
import assert from 'assert';

const exportIsFunction = typeof defaultExport === 'function';
assert(exportIsFunction, 'export is not a function');
assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxeBuilder === 'function', 'named export is not a function')
assert(defaultExport === AxeBuilder, 'default and named export are not the same');
2 changes: 2 additions & 0 deletions packages/puppeteer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
"scripts": {
"build": "tsup src/index.ts --dts --format esm,cjs",
"test": "mocha --timeout 60000 -r ts-node/register 'test/**.spec.ts'",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node test/esmTest.mjs",
"test:commonjs": "node test/commonjsTest.js",
"coverage": "nyc npm run test",
"prepublishOnly": "npm run build"
},
Expand Down
11 changes: 11 additions & 0 deletions packages/puppeteer/test/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// ensure backwards compatibility of commonJs format
const defaultExport = require('../dist/index.js').default;
const { AxePuppeteer } = require('../dist/index.js');
const assert = require('assert');

assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxePuppeteer === 'function', 'named export is not a function');
assert(
defaultExport === AxePuppeteer,
'default and named export are not the same'
);
13 changes: 7 additions & 6 deletions packages/puppeteer/test/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import defaultExport, { AxePuppeteer } from '../dist/index.mjs';
// ensure compatibility of ESM format
import defaultExport from '../dist/index.mjs';
import { AxePuppeteer } from '../dist/index.mjs';
import assert from 'assert';
import puppeteer from 'puppeteer';
import { fileURLToPath, pathToFileURL } from 'url';
import { join } from 'path';
import { fixturesPath } from 'axe-test-fixtures';

const exportIsFunction = typeof defaultExport === 'function';
const exportIsSame = defaultExport === AxePuppeteer;
assert(exportIsFunction, 'export is not a function');
assert(exportIsSame, 'default and named export is not the same');
assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxePuppeteer === 'function', 'named export is not a function')
assert(defaultExport === AxePuppeteer, 'default and named export are not the same');

const options = {};

Expand All @@ -30,4 +31,4 @@ async function integrationTest() {
await page.close();
await browser.close();
}
integrationTest();
integrationTest();
4 changes: 3 additions & 1 deletion packages/react/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
"build": "tsup index.ts --dts --format esm,cjs",
"prepare": "npm run build",
"test": "tsc && npm run test:types && jest",
"test:esm": "node esmTest.mjs",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node test/esmTest.mjs",
"test:commonjs": "node test/commonjsTest.js",
"test:types": "cd test && tsc"
},
"keywords": [
Expand Down
4 changes: 0 additions & 4 deletions packages/react/setupGlobals.mjs
View file
Open in desktop

This file was deleted.

8 changes: 8 additions & 0 deletions packages/react/test/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// ensure backwards compatibility of commonJs format
global.window = {};
global.document = {};

const defaultExport = require('../dist/index.js');
const assert = require('assert');

assert(typeof defaultExport === 'function', 'export is not a function');
6 changes: 4 additions & 2 deletions packages/react/esmTest.mjs → packages/react/test/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// ensure compatibility of ESM format

// in order to properly set global placeholders for `window` and `document` we have to
// import a file that does that.
// Setting them in this file will not work.
import _ from './setupGlobals.mjs';
import defaultExport from './dist/index.mjs';
import './setupGlobals.mjs';
import defaultExport from '../dist/index.mjs';
import assert from 'assert';

const exportIsFunction = typeof defaultExport === 'function';
Expand Down
2 changes: 2 additions & 0 deletions packages/react/test/setupGlobals.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
global.window = {};
global.document = {};
2 changes: 1 addition & 1 deletion packages/reporter-earl/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = {
transform: {
'\\.(ts|tsx)$': 'ts-jest'
},
testRegex: '/tests/.*\\.(ts|tsx|js)$',
testPathDirs: 'tests',
testPathIgnorePatterns: ['/node_modules/', '/dist/', '/tests/utils.ts'],
silent: false,
coverageThreshold: {
Expand Down
4 changes: 3 additions & 1 deletion packages/reporter-earl/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"scripts": {
"start": "NODE_OPTIONS=--experimental-vm-modules jest --watch --env=jsdom",
"test": "npm run build && npm run test:unit",
"test:esm": "node esmTest.mjs",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node tests/esmTest.mjs",
"test:commonjs": "node tests/commonjsTest.js",
"test:unit": "NODE_OPTIONS=--experimental-vm-modules jest --collectCoverage",
"build": "tsup src/axeReporterEarl.ts --dts --format esm,cjs",
"prepublishOnly": "npm run build"
Expand Down
6 changes: 6 additions & 0 deletions packages/reporter-earl/tests/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// ensure backwards compatibility of commonJs format
const defaultExport = require('../dist/axeReporterEarl.js').default;
const assert = require('assert');

const exportIsFunction = typeof defaultExport === 'function';
assert(exportIsFunction, 'export is not a function');
3 changes: 2 additions & 1 deletion packages/reporter-earl/esmTest.mjs → packages/reporter-earl/tests/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import defaultExport from './dist/axeReporterEarl.mjs';
// ensure compatibility of ESM format
import defaultExport from '../dist/axeReporterEarl.mjs';
import assert from 'assert';

const exportIsFunction = typeof defaultExport === 'function';
Expand Down
2 changes: 2 additions & 0 deletions packages/webdriverio/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
"prebuild": "rimraf dist",
"build": "tsup src/index.ts --dts --format esm,cjs",
"test": "mocha --timeout 60000 -r ts-node/register 'test/**.spec.ts'",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node test/esmTest.mjs",
"test:commonjs": "node test/commonjsTest.js",
"coverage": "nyc npm run test",
"prepare": "npm run build"
},
Expand Down
11 changes: 11 additions & 0 deletions packages/webdriverio/test/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// ensure backwards compatibility of commonJs format
const defaultExport = require('../dist/index.js').default;
const { AxeBuilder } = require('../dist/index.js');
const assert = require('assert');

assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxeBuilder === 'function', 'named export is not a function');
assert(
defaultExport === AxeBuilder,
'default and named export are not the same'
);
6 changes: 4 additions & 2 deletions packages/webdriverio/test/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import defaultExport from '../dist/index.mjs';
import { AxeBuilder } from '../dist/index.mjs';
import assert from 'assert';
import * as webdriverio from 'webdriverio';
import { fileURLToPath, pathToFileURL } from 'url';
import { join } from 'path';
import { fixturesPath } from 'axe-test-fixtures';

const exportIsFunction = typeof defaultExport === 'function';
assert(exportIsFunction, 'export is not a function');
assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxeBuilder === 'function', 'named export is not a function')
assert(defaultExport === AxeBuilder, 'default and named export are not the same');

async function integrationTest() {
const path = join(fixturesPath, 'index.html');
Expand Down
2 changes: 2 additions & 0 deletions packages/webdriverjs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@
"prebuild": "rimraf dist",
"build": "tsup src/index.ts --dts --format esm,cjs",
"test": "mocha --timeout 60000 -r ts-node/register 'test/**.spec.ts'",
"test:export": "npm run test:esm && npm run test:commonjs",
"test:esm": "node test/esmTest.mjs",
"test:commonjs": "node test/commonjsTest.js",
"coverage": "nyc npm run test",
"prepare": "npm run build"
},
Expand Down
7 changes: 7 additions & 0 deletions packages/webdriverjs/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,11 @@ export default class AxeBuilder {
}
}

// ensure backwards compatibility with commonJs default export
if (typeof module === 'object') {
module.exports = AxeBuilder;
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

module.exports.default = AxeBuilder;
module.exports.AxeBuilder = AxeBuilder;
}

export { AxeBuilder };
25 changes: 25 additions & 0 deletions packages/webdriverjs/test/commonjsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// ensure backwards compatibility of commonJs format
const implicitDefaultExport = require('../dist/index.js'); // support <4.7.3
const explicitDefaultExport = require('../dist/index.js').default; // support 4.7.3+
const { AxeBuilder } = require('../dist/index.js');
const assert = require('assert');

assert(typeof AxeBuilder === 'function', 'named export is not a function');

assert(
typeof implicitDefaultExport === 'function',
'implicit default export is not a function'
);
assert(
implicitDefaultExport === AxeBuilder,
'implicit default and named export are not the same'
);

assert(
typeof explicitDefaultExport === 'function',
'explicit default export is not a function'
);
assert(
explicitDefaultExport === AxeBuilder,
'explicit default and named export are not the same'
);
7 changes: 5 additions & 2 deletions packages/webdriverjs/test/esmTest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// ensure compatibility of ESM format
import defaultExport from '../dist/index.mjs';
import { AxeBuilder } from '../dist/index.mjs';
import assert from 'assert';

const exportIsFunction = typeof defaultExport === 'function';
assert(exportIsFunction, 'export is not a function');
assert(typeof defaultExport === 'function', 'default export is not a function');
assert(typeof AxeBuilder === 'function', 'named export is not a function')
assert(defaultExport === AxeBuilder, 'default and named export are not the same');