From e10d068e7333b0a2f9f19dda4d12969534b61c5c Mon Sep 17 00:00:00 2001 From: Shinrai Date: Sat, 3 Oct 2026 11:15:22 -0700 Subject: [PATCH] fix(cjs): require the ESM entry directly, fail clearly on Node without require(esm), and stop publishing devcheck index.cjs used createRequire(__filename) to synchronously load index.mjs. That idiom predates Node's native require(esm) and breaks bundlers (esbuild, webpack) that don't follow a createRequire-constructed require the way they follow a literal require() call. - index.cjs: a plain require("./index.mjs") instead of createRequire. Where Node.js has no require(esm) (before 20.19 / 22.12) it now throws ERR_REQUIRE_ESM with a message pointing to import() instead of a bare loader error. - index.mjs already avoided top-level await, so no change was needed there. - devcheck.mjs is a source-checkout-only dev warning and was never meant to ship: dropped "./devcheck" from exports and devcheck.mjs/types/devcheck.d.mts from files, and trimmed the now-stale devcheck globs out of bundle-size.yml's dist_paths. devcheck.mjs itself stays in the repo (index.mjs's fire-and-forget import of it already tolerates a missing file in the published package). - tests/cjs: node:test checks run by `npm test` and `npm run coverage` after Vitest: require() returns the same objects as import (type/identity only - droidsock() opens a real ADB connection, so no call is made), and the version check fires when require(esm) is off. test:cjs runs with CI=1 so devcheck's own NODE_OPTIONS guard (unrelated to this fix, source-checkout only) doesn't process.exit() the test runner. --- .github/workflows/bundle-size.yml | 2 +- index.cjs | 15 +++++++-- package.json | 11 ++---- tests/cjs/entry.test.cjs | 56 +++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 tests/cjs/entry.test.cjs diff --git a/.github/workflows/bundle-size.yml b/.github/workflows/bundle-size.yml index 03ced1d..c8082aa 100644 --- a/.github/workflows/bundle-size.yml +++ b/.github/workflows/bundle-size.yml @@ -40,7 +40,7 @@ jobs: with: build_command: "npm run build:ci" # Every glob starts with `*`: the v4.29.2 measure action walks each path prefix separately (a bare top-level file matches nothing, and mixing `*` globs with directory globs counts files twice). CLDMV/.github#326/#327 fix this upstream. - dist_paths: "*index.mjs,*index.cjs,*devcheck.mjs,*dist/**,*types/index.d.mts*,*types/devcheck*.mts,*types/dist/**" + dist_paths: "*index.mjs,*index.cjs,*dist/**,*types/index.d.mts*,*types/dist/**" # warning_pct: 5 # warning_bytes: 500 # comment_mode: "update" diff --git a/index.cjs b/index.cjs index 09de4f3..520f2b7 100644 --- a/index.cjs +++ b/index.cjs @@ -21,11 +21,20 @@ * * @module droidsock */ +"use strict"; -const { createRequire } = require("module"); -const requireESM = createRequire(__filename); +// index.cjs is a thin wrapper: it loads index.mjs through Node's synchronous require(esm). +// Node.js versions without require(esm) would fail with a bare ERR_REQUIRE_ESM, so fail +// early with a message that says what to do instead. +if (!process.features?.require_module) { + const error = new Error( + `@cldmv/droidsock: require() needs Node.js ^20.19.0 or >=22.12.0 (this is ${process.version}). On older Node.js, load the package with import() instead.` + ); + error.code = "ERR_REQUIRE_ESM"; + throw error; +} -const { default: droidsock } = requireESM("./index.mjs"); +const { default: droidsock } = require("./index.mjs"); // Export main function - the quick path, also callable with options module.exports = droidsock; // Default export diff --git a/package.json b/package.json index 6544d06..423172d 100644 --- a/package.json +++ b/package.json @@ -11,10 +11,6 @@ "import": "./index.mjs", "require": "./index.cjs" }, - "./devcheck": { - "types": "./types/devcheck.d.mts", - "import": "./devcheck.mjs" - }, "./main": { "droidsock-dev": { "types": "./types/src/droidsock.d.mts", @@ -32,10 +28,11 @@ "build": "node build.mjs", "build:types": "tsc --project .configs/tsconfig.dts.jsonc", "build:ci": "npm run build && npm run build:types && npm run test:types", - "test": "node tests/run-vitest.mjs", + "test": "node tests/run-vitest.mjs && npm run test:cjs", + "test:cjs": "CI=1 node --test tests/cjs/entry.test.cjs", "test:watch": "vitest --config .configs/vitest.config.mjs", "test:types": "tsc --noEmit --project .configs/tsconfig.dts.jsonc", - "coverage": "node tests/run-vitest.mjs --coverage-quiet", + "coverage": "node tests/run-vitest.mjs --coverage-quiet && npm run test:cjs", "ci:coverage": "npm run coverage", "lint": "eslint --config .configs/eslint.config.mjs .", "lint:fix": "eslint --config .configs/eslint.config.mjs . --fix", @@ -94,13 +91,11 @@ "files": [ "index.mjs", "index.cjs", - "devcheck.mjs", "README.md", "LICENSE", "types/dist/", "types/index.d.mts", "types/index.d.mts.map", - "types/devcheck.d.mts", "dist/" ], "sideEffects": false, diff --git a/tests/cjs/entry.test.cjs b/tests/cjs/entry.test.cjs new file mode 100644 index 0000000..d65442e --- /dev/null +++ b/tests/cjs/entry.test.cjs @@ -0,0 +1,56 @@ +/** + * + * @Project: @cldmv/droidsock + * @Filename: /tests/cjs/entry.test.cjs + * @Date: 2026-10-03T00:00:00-07:00 (1791010800) + * @Author: Nate Corcoran + * @Email: + * ----- + * @Last modified by: Nate Corcoran (Shinrai@users.noreply.github.com) + * @Last modified time: 2026-10-03T10:41:45-07:00 (1791049305) + * ----- + * @Copyright: Copyright (c) 2013-2026 Catalyzed Motivation Inc. All rights reserved. + * + */ + +/** + * CommonJS entry tests. These run under Node's own test runner (`node --test`), not Vitest: + * Vitest loads files through its own module runner, so it cannot show whether a plain + * `require()` of the package works the way it does for a CommonJS consumer. + */ +"use strict"; + +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const { spawnSync } = require("node:child_process"); +const path = require("node:path"); + +const repoRoot = path.resolve(__dirname, "../.."); + +test("require() returns the same droidsock object as import", async () => { + const droidsock = require("../../index.cjs"); + const esm = await import("../../index.mjs"); + + // droidsock() opens a real ADB connection when called, so only identity/type is + // checked here - no sockets are opened in this test. + assert.equal(typeof droidsock, "function"); + assert.equal(droidsock, esm.default); + assert.equal(droidsock.createDroidSock, esm.createDroidSock); + assert.equal(droidsock.DroidSock, esm.DroidSock); + assert.equal(droidsock.ADB, esm.ADB); + assert.equal(droidsock.AndroidDebugBridge, esm.AndroidDebugBridge); +}); + +test("require() fails with a clear message where Node.js has no require(esm)", () => { + // --no-experimental-require-module turns require(esm) off, which is what Node.js + // versions before 20.19 / 22.12 look like to the entry. + const res = spawnSync(process.execPath, ["--no-experimental-require-module", "-e", "require('./index.cjs')"], { + cwd: repoRoot, + encoding: "utf8" + }); + + assert.notEqual(res.status, 0); + assert.match(res.stderr, /ERR_REQUIRE_ESM/); + assert.match(res.stderr, /require\(\) needs Node\.js \^20\.19\.0 or >=22\.12\.0/); + assert.match(res.stderr, /import\(\)/); +});