Backend esm vitest#7605
Draft
SamTV12345 wants to merge 65 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>
Member
|
????? |
Member
Author
So far that works. So we can convert to ESM and use modern test runners like vitest + have actual editor integrations. With require everything is any. |
Brings 150 commits of develop work into the backend ESM + vitest migration
branch (v2.7.3 → v3.0.0 release prep). 41 conflicts resolved, preserving
this branch's ESM structure while layering in develop's new functionality.
Conflict resolution highlights:
- CHANGELOG.md: ESM/vitest plugin-author breaking changes folded into the
v3.0.0 entry as a new "Breaking changes for plugin authors" section.
- src/package.json: kept vitest-only test scripts, dropped mocha/mocha-froth
devDeps, took newer @types/node and etherpad-cli-client versions.
- src/node/db/{API,AuthorManager,DB,GroupManager,Pad}.ts: ESM exports
preserved, develop's new GDPR anonymize/search APIs, deletion-token flow,
ueberdb2 v6, and SYSTEM_AUTHOR_ID added as ESM exports.
- src/node/handler/{APIHandler,PadMessageHandler,RestAPI,SocketIORouter}.ts:
v1.3.1 API surface with compactPad/anonymizeAuthor, prometheus instrument
hooks, anonymizeIp + isAcceptingConnections imports, socketioServer rename
preserved.
- src/node/hooks/express/*: ESM imports, anonymizeIp/ensureAuthorTokenCookie/
socialMeta wired in for v3 features.
- src/node/utils/{Settings,UpdateCheck,ImportEtherpad}.ts: createHash for
randomVersionString determinism, axios dropped in favour of native fetch /
undici, dynamic ueberdb2 import removed (static import works under ESM).
- src/node/server.ts: undici ProxyAgent (axios removal from develop), pkg.json
import with `with { type: 'json' }` ESM attributes.
- src/static/js/{ace,pad,pad_editor,pad_userlist,timeslider}.ts +
pluginfw/installer.ts: full require()→import conversion preserved,
develop's pad_mode/showPrivacyBannerIfEnabled/pad_version_badge/
pluginCatalogGuard/pluginEngineCheck/installerTasks imports added,
pad_editor's focusOnLine settle-loop refactor kept.
- Backend test specs: ESM imports + .js extensions preserved, vitest harness
in src/tests/backend/common.ts kept (with develop's diagnostic loggers
layered in), Settings CJS-compat tests dropped (no longer applicable
under ESM), randomVersionString + padOptions tests converted to ESM.
- Frontend specs: import unions resolved with .js extensions.
- fuzzImportTest.ts: kept deleted (removed earlier in this branch with
mocha-froth).
- .github/workflows/backend-tests.yml: unified single `pnpm test` (vitest)
with develop's Node diagnostic-report capture.
- pnpm-lock.yaml: taken from develop; will need `pnpm install` to reconcile
against the merged package.json (drops mocha/mocha-froth lockfile entries).
After the develop merge, the new files from develop's side weren't
ESM-converted (this branch's mass require()→import + .js-extension pass
predated them). 271 tsc errors revealed across 87 files.
- src/node/db/DB.ts: restored closing `}` for the `if (db.metrics != null)`
block that the merge conflict resolution dropped — was the only genuine
merge-resolution bug.
- src/node/db/PadDeletionManager.ts: convert `exports.foo` → `export const`,
`require('./DB')` → `import DB from './DB.js'`.
- src/node/hooks/i18n.ts: widen `locales` type to
`{[lang]: {[key]: string}}` so renderSocialMeta's call sites typecheck.
- src/node/updater/**/*.ts, hooks/express/{openapi-admin,updateActions,
updateStatus}.ts, prom-instruments.ts, prometheus.ts, utils/SkinColors.ts,
utils/ensureAuthorTokenCookie.ts, static/js/pluginfw/pluginCatalogGuard.ts:
add `.js` extensions to relative imports (static + dynamic). Fix the
one over-broad fix: `'../../updater'` → `'../../updater/index.js'`
(explicit dir-index import under ESM).
- tests/backend-new/**, tests/backend/specs/**, tests/frontend-new/**:
add `.js` extensions to imports across develop's new test specs.
- tests/backend/specs/{admin,api,*}.ts: add `this: any` annotation to
mocha-style `function () { this.timeout(N); ... }` callbacks so they
typecheck under vitest. Runtime semantics unchanged — `this.timeout()`
was already a no-op under the vitest harness (no mocha context shim).
`pnpm run ts-check` now passes with zero errors.
Replaces the `this: any` annotations introduced in the previous commit with
a real migration to vitest's native API. The previous fix only made tsc
green; this fix makes the test contract correct under vitest.
mocha → vitest mappings:
- `before(function () { this.timeout(N); ... })` → `before(async () => { ... })`.
vitest.config.ts already sets `hookTimeout: 60000` and `testTimeout: 120000`,
so the per-hook overrides were redundant. (The longer mocha-style timeout
could be passed as the second arg to before(): `before(async () => {}, ms)`.
None of the call sites needed it.)
- `describe('foo', function () { this.timeout(N); ... })` → `describe('foo', () => { ... })`.
Same reason — covered by testTimeout default.
- `it(..., function () { try { require.resolve('html-to-docx'); } catch { this.skip(); } ...})`
→ hoist `const hasHtmlToDocx = canResolve(...)` to module load, then
`describe.skipIf(!hasHtmlToDocx)(...)` or `it.skipIf(!hasHtmlToDocx)(...)`.
Vitest evaluates skipIf at definition time, so this works for the
require.resolve-style "skip if optional dep missing" pattern that drives
every skip in export.ts and import.ts.
- For the one runtime-only condition (heading-block plugin registration via
ccRegisterBlockElements, which needs common.init() to have run): keep a
closure flag set in `before(...)` and call `ctx.skip()` inside each `it`
that needs it. vitest exposes `skip` on the test-context parameter.
- `function ()` callbacks bulk-converted to arrow functions across the touched
files now that no body references `this`.
- Removed `src/tests/backend/diagnostics.ts` — it was a `mocha --require` hook
shim (mochaHooks export, this.currentTest access). Mocha was dropped from
this branch's devDeps; nothing imports it, vitest.config doesn't load it.
`pnpm run ts-check` passes with zero errors. No `this: any` anywhere in the
test tree.
The previous merge resolution caught the obvious `require()` calls in
files with conflict markers, but several develop-side files use bare
`require()` or `exports.foo = ...` outside of conflict zones. These
compile fine under tsc (which is permissive about CJS-shape syntax in
ESM modules) but throw at runtime as soon as the module is loaded by
Node's ESM loader: `ReferenceError: require/exports is not defined in
ES module scope`.
That's what the CI surfaced — every job that boots the server or builds
the admin UI (which imports the server modules to dump the OpenAPI
spec) was failing on:
src/node/utils/Settings.ts:1346 `require('../../package.json')`
src/node/hooks/express/openapi.ts:874 `exports.generateDefinitionForVersion = ...`
Fixed:
- Settings.ts:1346 — replaced the inline `require(...).version` with the
existing ESM-safe `getEpVersion()` helper (which already uses the
module's createRequire bridge on line 884). Same value, no duplication.
- server.ts:193 — `require('./updater')` → `await import('./updater/index.js')`
for the optional markBootHealthy() call.
- ExportHandler.ts — added a createRequire bridge and replaced the four
inline `require()` calls (sofficeAvailable, ExportSanitizeHtml,
html-to-docx, ExportPdfNative) with ESM imports where possible.
- ImportHandler.ts — added static ESM imports for ImportDocxNative and
ExportSanitizeHtml; dropped the three inline `require()` calls.
- ExportPdfNative.ts, ImportDocxNative.ts — added createRequire bridges
for the top-level CJS-only npm package requires (pdfkit, mammoth, jszip).
These packages publish CJS entries that don't round-trip cleanly through
ESM default-import interop under tsx; the bridge is the minimum-risk fix.
- adminsettings.ts — replaced inline `require('AuthorManager')` with a
static `import * as authorManager`.
- pad_editbar.ts — replaced `require('./pad_mode')` with `await import(...)`
inside the showTimeSlider click handler (which is already async-capable).
- openapi.ts:874-875 — `exports.X = X; exports.Y = Y;` → `export {X, Y};`.
- openapi-admin.ts — dropped the redundant `exports.expressPreSession =
expressPreSession;` (the function is already `export const`) and the
duplicate `exports.generateAdminDefinition = generateAdminDefinition;`
(also already `export const` on the declaration line).
Verified locally:
- `pnpm run ts-check` passes.
- `admin/scripts/dump-spec.ts` — the script that booted the failing
`dump-spec.ts failed with exit code 1` in CI — now runs to completion
and writes a 128 KB spec.
CI Feedback 🧐(Feedback updated until commit ad66e94)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.