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

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

Abierto
#113 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
TypeScript
Estrellas
5
Forks
10
Merge medio
3 h 23 min
PR fusionados (30 d)
3

Descripción

## 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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.