apex-dev-tools / apex-dev-tools/benchmarker

Switch to module nodenext and Node 22.12+ to unblock ESM-only dependencies (precursor to #95)

オープン
#113 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
TypeScript
スター
5
フォーク
10
平均マージ
3時間 23分
マージ済み PR(30日)
3

説明

## Problem

The package is CommonJS (`package.json` has no `type` field, `tsconfig.json` has `module: node16`, TypeScript 6.0.3, `engines.node >20.0.0`). TypeScript refuses to compile a static `import` of an ESM-only package from a CommonJS module under `node16`, so every ESM-only dependency major is currently unmergeable:

- **#108** puppeteer 24.43.1 → 25.9.0: Build fails with 12× `TS1479` (7 files in `src/`, 4 in `test/`) plus 1× `TS1541` in `src/shared/env.ts` (type-only import needs a `resolution-mode` attribute). Run 33741760401.
- **#102** chai 6.2.2 / chai-as-promised 8.0.2 / sinon-chai 4.0.1 (+ `@types`): merged, broke `main` with the same `TS1479`, reverted in #111. Dependabot now ignores chai-stack majors.

#95 proposes a full dual ESM/CJS build to fix this. That is a large change to the published contract. There is a much smaller first step.

## Proposal

1. `tsconfig.json`: `module: node16` → `module: nodenext`.
2. `package.json`: `engines.node` → `>=22.12.0`.

Node 22.12+ supports `require(esm)` natively and without a flag, and TypeScript ≥ 5.8 under `nodenext` type-checks a CommonJS `import` of an ESM package as a `require()` call. The emitted output stays CommonJS, `main`/`exports` do not change, and no bundler is needed. Puppeteer 25 already declares `engines.node >=22.12.0`, so the floor bump is unavoidable for #108 regardless.

## Verified findings

Environment: Node v22.22.0, npm 10.9.4, TypeScript 6.0.3, macOS. Scratch branch on top of #108 (`puppeteer 25.9.0`), discarded afterwards.

**Build**

- Baseline (`module: node16`): reproduced all 13 errors from CI.
- `module: nodenext`, nothing else changed: `npm run build` clean, 0 errors. The `TS1541` in `env.ts` also disappears; no `resolution-mode` attribute needed.
- Then re-applied the #102 versions on top (chai 6.2.2, chai-as-promised 8.0.2, sinon-chai 4.0.1, @types/chai-as-promised 8.0.2, @types/sinon-chai 4.0.0; @types/chai left at 4.3.20 as in #102): still 0 errors.
- Emitted `dist/` is still CommonJS (`"use strict"`, `Object.defineProperty(exports, ...)`, `require("...")`).
- The `PuppeteerNodeLaunchOptions` → `LaunchOptions` rename mentioned in #95 is already absorbed on `main` (`src/shared/env.ts` imports `LaunchOptions`). Nothing to do.
- ts-patch transform (`scripts/apexLoader.ts`) still runs under `nodenext` (`Inlining apex: .../GovernorLimits.apex` printed, string inlined). typeorm `experimentalDecorators` still emit via `tslib.__decorate`. `npx typedoc` still generates (0 errors, 14 pre-existing warnings). `eslint ./src ./test ./test_system`: 0 errors, 6 pre-existing warnings.

**Runtime `require(esm)`**

- puppeteer 25.9.0 and puppeteer-core 25.9.0 are `"type": "module"`, `engines.node >=22.12.0`. The `exports["."]` map points both `import` and `require` at the same ESM file `lib/puppeteer/puppeteer.js`, so it relies on `require(esm)`.
- `node -e "require('puppeteer')"` on 22.22.0 succeeds, `launch` is a function, no `ERR_REQUIRE_ASYNC_MODULE`, no warning printed. The entry graph has no top-level await.
- `require('chai')`, `require('chai-as-promised')`, `require('sinon-chai')` all succeed.
- `npm ls extract-zip` is empty with puppeteer 25 (the Dependabot alert dependency is gone from the tree). Remaining `npm audit` entries are `diff` and `mocha`, unrelated.

**Tests (`npm test`)**

- Result with `nodenext` + puppeteer 25, identical with chai 4.5.0 and with chai 6.2.2: **254 passing, 19 pending, 1 failing**. Coverage thresholds met.
- The failure is real and caused by puppeteer 25 being ESM, not by `nodenext` or chai:
```
test/testTemplates/formLoadWithDataLoadingTestTemplate.test.ts:48
stub(puppeteer, 'launch') → TypeError: ES Modules cannot be stubbed
```
`require(esm)` returns a sealed module namespace object, so sinon cannot replace `launch` on it. This is the only place in `test/` that stubs a puppeteer export. Because it is a `beforeEach` hook, the rest of that `describe` is skipped, which is why the count differs from the 257 passing on `main`. Fix: route `launch()` in `src/testTemplates/formLoadTestTemplate.ts` and `formLoadWithDataLoadingTestTemplate.ts` through a small internal wrapper module and stub that instead.
- Loader interaction: the `--loader ts-node/esm --experimental-specifier-resolution=node` in `NODE_OPTIONS` is what actually carries the suite. mocha 11 loads test files via `import()`; Node 22 strips types from `.ts` natively, so without the loader the run fails immediately with `ERR_MODULE_NOT_FOUND` on the first extensionless relative import (`src/database/alertInfo`). `ts-node/register` in `.mocharc.yml` is effectively redundant for `.ts` files. This is pre-existing and unchanged by `nodenext`; both work as-is. Node prints `ExperimentalWarning: --experimental-loader may be removed in the future; instead use register()`. Moving to the `--import` + `module.register('ts-node/esm')` form is an optional follow-up, not required here.

**Versions / CI**

- `@types/node` latest 22.x is 22.20.1. Currently 20.19.43 and Dependabot ignores majors for it, so it needs one manual bump.
- `Build.yml` and `Publish.yml` already use `actions/setup-node@v7` with `node-version: 22`, which resolves to the latest 22.x (≥ 22.12), so they already satisfy the floor. No change strictly required.
- `lint-staged@17.4.1` declares `node >=22.22.1`; npm warns `EBADENGINE` on 22.22.0. Unrelated to this change but relevant when picking the floor wording.

## Scope of change

| File | Change |
|---|---|
| `tsconfig.json` | `module: "node16"` → `"nodenext"` |
| `package.json` | `engines.node` → `>=22.12.0`; `puppeteer` 25.9.0 (take #108); chai 6.2.2, chai-as-promised 8.0.2, sinon-chai 4.0.1, @types/chai-as-promised 8.0.2, @types/sinon-chai 4.0.0 (re-apply #102); `@types/node` → 22.x; version → 8.0.0 (see below) |
| `package-lock.json` | regenerate |
| `src/testTemplates/formLoadTestTemplate.ts`, `formLoadWithDataLoadingTestTemplate.ts` | call `launch` through an internal wrapper (e.g. `src/shared/browser.ts`) so tests can stub it |
| `test/testTemplates/formLoadWithDataLoadingTestTemplate.test.ts` | stub the wrapper instead of `puppeteer.launch` |
| `.github/dependabot.yml` | remove the chai-stack major ignore block and its comment; keep the `@types/node` ignore |
| `.github/workflows/Build.yml`, `Publish.yml` | no change needed (`node-version: 22` already ≥ 22.12). Optionally pin `22.12` or later explicitly for clarity |
| `CHANGELOG.md`, `docs/user` | document the Node 22.12 minimum |
| `.mocharc.yml` / `test` script | no change required; optional cleanup of the redundant `ts-node/register` and the deprecated `--loader` form |

## Out of scope

The dual ESM/CJS build stays in #95. After this change:

- The dependency-unblocking motivation for #95 is gone (both blocked stacks build and load).
- The codebase already type-checks under `nodenext`, `moduleResolution` is settled, and the ESM/CJS import audit for puppeteer/chai is done.
- What remains for #95 is purely the publishing contract: adding an ESM entry point (`"type"`/`exports.import` condition, an ESM emit, `.d.ts`/`.d.cts` split) and reconciling the test runner. #95 can be re-scoped to that and reprioritised.

## Risks and open questions

- **Node 20 consumers cannot take this release.** `engines` moves from `>20.0.0` to `>=22.12.0`. Node 20 reached end-of-life on 2026-04-30, but any consumer still on it is pinned to 7.x. This is a breaking change to the supported runtime and should ship as **8.0.0**.
- **Published type declarations import from `puppeteer`.** `dist/types/src/shared/env.d.ts`, `uiHelper.d.ts`, `services/metrics.d.ts`, `services/navigate*.d.ts` and both form-load template `.d.ts` files import puppeteer types, and `Env`, `UiHelper` and the templates are re-exported from `index`. A consumer compiling with `module: node16`/`commonjs`, TypeScript < 5.8 and `skipLibCheck: false` would see `TS1479`/`TS1541` inside our declarations. Consumers on TS ≥ 5.8 with `nodenext`, or with `skipLibCheck: true`, are unaffected. Worth a note in the changelog; confirm how downstream projects compile.
- **`require(esm)` moves a class of failure from compile time to run time.** If a future version of puppeteer (or a transitive dependency) introduces top-level await, `require` throws `ERR_REQUIRE_ASYNC_MODULE` at load rather than `tsc` failing. The unit tests exercise the imports, so CI still catches it.
- **`target: es2020`** can stay for now; raising it to `es2022` is a separate, safe follow-up once Node 22 is the floor.
- The `--experimental-loader` deprecation warning is noise today; Node may eventually remove the flag, at which point the `--import`/`register()` form becomes mandatory.

## Acceptance criteria

- [ ] `tsconfig.json` uses `module: nodenext`; `engines.node` is `>=22.12.0`; version bumped to 8.0.0 with changelog entry.
- [ ] #108 (puppeteer 25.9.0) rebased onto this change builds green.
- [ ] The chai 6 / chai-as-promised 8 / sinon-chai 4 bump (versions from #102) builds green and the Dependabot ignore rules from #111 are removed.
- [ ] `npm test` passes fully (257 tests as on `main`), including the `formLoadWithDataLoadingTestTemplate` suite with `launch` stubbed via a wrapper.
- [ ] `npm run build` output in `dist/` is still CommonJS and `npm run doc:generate` succeeds.
- [ ] The extract-zip Dependabot alert clears (`npm ls extract-zip` empty).
- [ ] `@types/node` on 22.x.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。