Backend esm vitest#7605
Draft
SamTV12345 wants to merge 101 commits into
Draft
Conversation
Converted: customError, Stream, NodeVersion, checkValidRev, AbsolutePaths (+__dirname shim), run_cmd, Cleanup, ExportHelper, padDiff, ExportTxt, LibreOffice, UpdateCheck, ImportHtml. Still CJS in utils/: Settings, Minify, toolbar, ExportEtherpad, ImportEtherpad, ExportHtml. Their consumers will surface errors until they're flipped too. ts-check: 530 -> 526 errors.
…ad, ImportEtherpad, ExportHtml, toolbar) All 26 files in node/utils/ now ESM. Settings.ts CJS shim removed (dead code under "type": module); plugins doing require() get .default-wrapped namespace via Node interop, the createRequire bridge in pluginfw will preserve sync loading once that lands. toolbar.ts's module.exports.availableButtons self-reference replaced by a module-private toolbar const. ts-check: 526 -> 539 errors. The +13 is from default-imports landing on modules that still live in db/ as CJS; resolves once db/ is flipped.
Self-referential exports.X pattern replaced by a module-private `eejs` object that's also the default export. __dirname shimmed via import.meta.url. `require` (used by templates as args.require) preserved via createRequire.
DB.ts: wrapped mutable exports as default-exported `dbModule` so init() can
still re-bind get/set/findKeys at runtime.
AuthorManager / GroupManager / PadManager / SessionManager: exports.X -> export
const X. Internal self-references (exports.X(...)) replaced with bare X(...).
Aliases (doesAuthorExists, doesPadExists, getAuthor4Token-deprecated)
preserved.
SecurityManager / ReadOnlyManager: imports flipped to .js.
SessionStore: module.exports -> export default.
Pad: `exports.cleanText` and `exports.Pad = Pad` now `export const cleanText`
and `export { Pad }`. Internal exports.cleanText() calls -> cleanText().
API: ~40 `exports.X = ...` rewritten to `export const X = ...`.
crypto.ts: util.promisify wrappers exported as named consts. SecretRotator: import flips, all internal refs already named. OIDCAdapter / OAuth2Provider / OAuth2User: already ESM, just .js suffixes.
APIHandler / APIKeyHandler / SocketIORouter / ExportHandler / ImportHandler / RestAPI: imports flipped to .js, exports.X -> export const X. PadMessageHandler: full conversion incl. internal exports.X() callsites (sendChatMessageToPadClients, updatePadClients, composePadChangesets) rewritten to bare names. Default export object added so existing `import padMessageHandler from '...'` callers keep working without changes. Conditional require() of LibreOffice in ImportHandler/ExportHandler hoisted to a top-level namespace import (`import * as converterModule`).
…s/* files) Done: express.ts (incl. https/http hoisted to top-level imports, exports.server + exports.sessionMiddleware -> export let), i18n.ts, admin.ts, webaccess.ts (authnFailureDelayMs preserved as export let + setter), padurlsanitize.ts, pwa.ts, errorhandling.ts (exports.app preserved as export let), tokenTransfer.ts, adminplugins.ts. Still CJS in hooks/express/: adminsettings, apicalls, importexport, openapi, socketio, specialpages, static.
…tsort to ESM, add createRequire bridge in shared.ts
…s, padaccess) to ESM
…toreRevision, instance, health, fuzzImportTest Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ctor, export_list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…y, sanitizePluginsForWire, messages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gression-db, lowerCasePadIds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ebaccess, undo_clear_authorship, specialpages, socketio Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Change 'import common from' to 'import * as common from' in 20 test files to use named imports instead of default import. Co-authored-by: GitHub Copilot <copilot@github.com>
Replace default imports with namespace imports for modules that only export named exports: - PadManager.ts: export const getPad, listAllPads, etc. - AuthorManager.ts: export const getAuthor, etc. - ImportHtml.ts: export const setPadHTML - ExportHtml.ts: export const getPadHTMLDocument Changed 'import X from Y' to 'import * as X from Y' in: - Test files (export_list, chat, messages, etc.) - Utility files (ExportHtml, ExportTxt, ExportEtherpad, ImportEtherpad, Cleanup) - API test files (pad, restoreRevision) This fixes ESM module resolution errors when these modules are imported as default exports despite only providing named exports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Convert const {A, B} = require('Y') to import {A, B} from 'Y.js'
- Add .js extensions to relative imports
- Keep external packages without .js (e.g., 'tinycon/tinycon')
- Convert exports.X = Y to export {X}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Convert const {A, B} = require('Y') to import {A, B} from 'Y.js'
- Add .js extensions to relative imports
- Keep external packages without .js (e.g., 'tinycon/tinycon')
- Convert exports.X = Y to export {X}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Convert const {A, B} = require('Y') to import {A, B} from 'Y.js'
- Add .js extensions to relative imports
- Keep external packages without .js (e.g., 'tinycon/tinycon')
- Convert exports.X = Y to export {X}
- Update self-references to avoid circular dependency issues
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Convert const {A, B} = require('Y') to import {A, B} from 'Y.js'
- Add .js extensions to relative imports
- Keep external packages without .js (e.g., 'tinycon/tinycon')
- Convert exports.X = Y to export {X}
- Refactor forward references to allow function reordering
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Convert const {A, B} = require('Y') to import {A, B} from 'Y.js'
- Add .js extensions to relative imports
- Keep external packages without .js (e.g., 'tinycon/tinycon', 'underscore')
- Convert exports.X = Y to export {X}
- Convert module.exports to export default
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert const X = require('Y') to import X from 'Y.js'
- Add .js extensions to relative imports
- Convert exports.X = Y to export {X}
- Convert dynamic requires to dynamic imports for circular dependency handling
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert dynamic requires to dynamic imports for circular dependency handling - Make timeslider.init async to support dynamic imports - Update exports references to use module scope or window - Add ESM export default for jquery library - Keep CommonJS compatibility for jquery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- pad_automatic_reconnect.ts: export const showCountDownTimerToReconnectOnModal
- pad_cookie.ts: convert to named export with const class instance
- pad_impexp.ts: export {padimpexp}
- pad_savedrevs.ts: export const saveNow, export const init
- skin_variants.ts: export multiple functions
- changesettracker.ts: export {makeChangesetTracker}
- broadcast_revisions.ts: export {loadBroadcastRevisionsJS}
- AttributeManager.ts: add .js extensions to imports, export default AttributeManager
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Convert remaining exports.baseURL references to use the baseURL const defined at module level. This completes the conversion from CommonJS require/exports to ESM import/export syntax. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 25, 2026
CJS plugins frequently do `require('ep_etherpad-lite/node/db/PadManager')`
or `require('.../node/db/DB').db.set(...)`. Two things blocked this on the
ESM branch and one new thing was needed for it to work end-to-end:
1. DB.ts and ImportEtherpad.ts had a top-level `import {Database} from
'ueberdb2'`. ueberdb2 v6 is ESM-only — no `require` export condition —
so the CJS twin's `require('ueberdb2')` crashed with "No 'exports' main
defined". Convert both to `import type` (erased) plus a lazy
`await import('ueberdb2')` inside the call sites (init() in DB.ts, the
`new Database('memory', ...)` site in ImportEtherpad.ts).
2. Node's module cache treats the ESM source and the CJS twin as two
separate module records. Etherpad's startup calls init() on the ESM
record only; the plugin's CJS-required dbModule would otherwise have
its own `db: null` forever. Stash the live `db` handle and the
wrapper-method closures on globalThis under a private key; replace
dbModule with a Proxy that reads/writes through the shared store.
The Proxy also routes writes for the wrapper names (`set`, `get`, …)
into the shared store, so existing tests that monkey-patch
`dbModule.set = ...` (e.g. regression-db.ts, SessionStore.ts)
continue to see their stub from both module records.
3. prom-instruments.ts unconditionally constructs three prom-client
metrics at load time and registers them with the default Registry.
When the CJS twin loads (because a plugin transitively pulls it via
PadMessageHandler → prom-instruments), the second registration throws
"metric with the name X has already been registered". Wrap each
metric construction with a `getSingleMetric()` lookup so re-loads
reuse the existing handle instead of crashing.
Then turn the exports-map `./node/db/*` and `./node/db/*.js` entries
from import-only into dual-condition (import + require), re-enable
`node/db/**` and `node/utils/ImportEtherpad.ts` in tsdown's CJS entry
list, and update exports_map.ts to assert that db subpaths are now
require-able.
Local test result: 2220 passed | 25 skipped | 0 failed (parity with
pre-change baseline). New tests verify that
require('ep_etherpad-lite/node/db/PadManager') no longer throws.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plugins call eejs.require('./templates/x.html', {}, module) where `module`
is the CJS module global of the calling plugin. Some module-loader shims
do not populate .filename — notably vite-node's CJS shim, and the
Module._extensions['.ts'] handler in pluginfw/plugins.ts for plugins
that ship .ts files (e.g. ep_markdown). Calling path.dirname(undefined)
threw TypeError and surfaced as 500 Internal Server Error on every pad
page that rendered an affected plugin's eejsBlock_* template — broke
3 socketio.ts tests under WITH_PLUGINS once ep_markdown started loading
successfully (it used to crash earlier in the load chain).
Skip the mod.filename branch when filename isn't a string; keep the
existing basedir (file_stack top or __dirname) and adopt mod.paths
only if it's actually an array.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ep_markdown@12.0.12 (and other plugins using ep_plugin_helpers/eejs)
call eejs.require('./templates/x.html', {}, module). In vite-node's
CJS-in-ESM bridge and the .ts CJS-extension handler in pluginfw/plugins.ts,
the `module` shim handed to the plugin doesn't have .filename populated.
Previously this threw TypeError on path.dirname(undefined); my last commit
silenced the throw but left basedir at the eejs file's own directory,
which then failed resolve.sync with "Cannot find module './templates/x.html'
from .../dist-cjs/node/eejs". 500 Internal Server Error on every pad
page that touches an affected plugin's eejsBlock_* hook.
When the relative basedir is genuinely unusable (no module.filename
AND no outer template on file_stack), walk the JS Error stack to find
the first frame outside eejs — that's the plugin's source file — and
use its directory. Defensive coding for an upstream-shim shortcoming;
the path.dirname-of-undefined throw is also gone for good.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plugins sometimes schedule deferred custom-message emits via setTimeout — ep_set_title_on_pad's debounced rename queue is the immediate example — and those timers can outlive the server. In test shutdown (and during graceful restarts in production) socketioServer is unset before the queued timer fires; reaching into its `.sockets` throws "Cannot read properties of null" which surfaces as an uncaughtException and kills the vitest worker even after every test in the file has passed. CI failure mode on PR #7605: backend tests showed `Test Files 48 passed, Tests 1230 passed | 0 failed, Errors 1` — the worker exited unexpectedly because of this exact stack: TypeError: Cannot read properties of null (reading 'sockets') at handleCustomObjectMessage (PadMessageHandler:476:18) at Timeout._onTimeout (ep_set_title_on_pad/index.js:49:23) Add an early no-op when socketioServer (or its .sockets) is null. The intended recipients are gone too; dropping the message is the correct behavior in that window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
specialpages.ts uses esbuild's buildSync() with default browser platform to bundle client-side JS at server startup. Browser platform honors the conditions ['browser', 'default', ...] — NOT ['import', 'require']. Our exports map only had import + require, so every 'ep_etherpad-lite/static/js/*' reference in the client bundle failed to resolve, exploding into ~12 'Could not resolve' errors and killing the Playwright Firefox/Chrome jobs at boot. Add a 'default' fallback to every conditional exports entry pointing to the same .mjs target as the import condition. 'default' is the catch-all and is consulted after import/require, so it doesn't change behavior for Node ESM or CJS consumers; it only adds matches for esbuild's browser mode and similar non-node resolvers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`pnpm run prod` boots etherpad which calls esbuild.buildSync() during client-JS bundling. esbuild resolves ep_etherpad-lite/static/js/* refs through the package's exports map, which points at dist/dist-cjs files. Without a preceding build those files don't exist on CI, and the boot fails with ~12 "Could not resolve" errors — which is why all four Playwright jobs (.github/workflows/frontend-tests.yml) and the upgrade-from-release job failed even after my prior exports-map fix. Add a `preprod` script mirroring `pretest`/`predev`. Anyone running `pnpm run prod` from a clean tree gets a usable dist/ + dist-cjs/ without needing a workflow-side build step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ui/src/main.ts imports `from 'ep_etherpad-lite/node/types/MapType.js'` for a single type-only reference. tsc resolves through the exports map, ends up at dist/node/types/MapType.mjs (no .d.ts sibling, since tsdown runs with dts:false), and gives up with TS7016 — "Could not find a declaration file for module". This crashed the Docker build step that the rate-limit job uses for its container image (see Dockerfile line 25: 'RUN pnpm run build:ui'). Add a `types` condition on every wildcard/exact exports entry, pointing to the source .ts file. tsc picks `types` before any other condition when looking for declarations, so type-only imports find the .ts and resolve cleanly without needing a generated .d.ts. Also fixes the ui/src/main.ts import that ended in '.ts' (incorrect per TS conventions; should be '.js' even when the source is .ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three issues surfaced by the ESM migration that pre-existing CI exercised:
1. Dockerfile CMD used `node --require tsx/cjs ...`. tsx/cjs is the
CJS-only hook — it doesn't intercept Node's native ESM resolver.
server.ts is now an ESM module (uses import/export syntax), so on
the first `import './foo.js'` Node tries to load a literal .js
file off disk and crashes with ERR_MODULE_NOT_FOUND
(Cannot find module .../node/types/Plugin.js). Switch to
`--import tsx`, which registers ESM-aware loader hooks. This is the
immediate cause of the rate-limit job's docker container exiting
on boot ("No such container: etherpad-docker").
2. The same docker image never ran `pnpm run build`. The runtime
esbuild-bundles the client JS at server startup and resolves
`ep_etherpad-lite/static/js/*` through the package's exports map,
which now points at dist/ + dist-cjs/. Without those directories
the bundle fails with 12+ "Could not resolve" errors. Add
`RUN cd src && pnpm run build` after dependencies are installed so
the dist surface is baked into the image.
3. The test-admin script in src/package.json still passed
`tests/frontend-new/admin-spec --project=chromium`, but the
playwright.config.ts merged in from develop moved admin specs under
a dedicated `chromium-admin` project (testMatch =
'tests/frontend-new/admin-spec/**'). The chromium project's
testMatch is the regular frontend specs, so Playwright filtered to
zero matching tests and exited with "Error: No tests found",
crashing the Frontend admin tests workflow. Align with develop:
drop the redundant path arg, use --project=chromium-admin.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tsdown is in src/'s devDependencies, which installDeps.sh strips in the production stage. Move the 'pnpm run build' invocation into the adminbuild stage (which keeps all deps) and COPY --from=adminbuild the resulting dist + dist-cjs into the production image. Previous attempt ran the build in the production stage after installDeps.sh and crashed with 'sh: tsdown: not found'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CJS branch
jQuery's UMD wrapper at the top of vendors/jquery.ts has two branches:
if (typeof module === "object" && typeof module.exports === "object") {
module.exports = factory(global, true); // noGlobal=true
} else {
factory(global); // sets window.jQuery
}
The `factory(global, true)` form skips its own
`window.jQuery = window.$ = jQuery` assignment (the `if (typeof noGlobal
=== "undefined")` guard inside the factory rejects when noGlobal is true).
That was fine in develop's CJS world because consumers did
`require('./vendors/jquery')` and got the function via module.exports.
In our ESM/bundled world, specialpages.ts's runtime esbuild wraps every
module in a CJS-style shim — `typeof module === "object"` becomes truthy
inside the IIFE — so the CJS branch fires, but the ESM consumer
(rjquery.ts) reads `window.jQuery` and finds it undefined:
Error: Failed to initialize jQuery from ./vendors/jquery.js
at rjquery.ts → cascades into every iframe-create path → no editor
iframe → frontend tests time out waiting for ace_outer.
Fix is purely defensive: after the IIFE, if window.jQuery is still
missing AND module.exports holds the jQuery function (which it does on
the CJS branch), promote it onto window. Original behavior is preserved
on the non-CJS branch (window.jQuery already set, conditional no-ops).
Also fix the export-default expression: the old `typeof window.$ ===
"object"` was wrong — jQuery is a function, not an object — and would
have returned null even if globals were set. Replace with `window.$ ??
null`.
Reproduced locally: a11y_dialogs.spec.ts goes from 0/20 timing out to
20/20 passing. The whole Playwright failure on PR #7605 collapses to
this single root cause.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.