Skip to content

Commit ad66e94

Browse files
committed
fix: eliminate runtime require()/exports.foo bombs in ESM modules
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.
1 parent 95f753c commit ad66e94

10 files changed

Lines changed: 32 additions & 20 deletions

File tree

src/node/handler/ExportHandler.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,25 @@
2020
* limitations under the License.
2121
*/
2222

23+
import {createRequire} from 'node:module';
2324
import * as exporthtml from '../utils/ExportHtml.js';
2425
import * as exporttxt from '../utils/ExportTxt.js';
2526
import * as exportEtherpad from '../utils/ExportEtherpad.js';
2627
import fs from 'fs';
27-
import settings from '../utils/Settings.js';
28+
import settings, {sofficeAvailable} from '../utils/Settings.js';
29+
import * as ExportSanitizeHtml from '../utils/ExportSanitizeHtml.js';
2830
import os from 'os';
2931
import hooks from '../../static/js/pluginfw/hooks.js';
3032
import util from 'util';
3133
import { checkValidRev } from '../utils/checkValidRev.js';
3234
import * as converterModule from '../utils/LibreOffice.js';
3335

36+
// Lazy CJS bridge for optional native-export modules (html-to-docx,
37+
// ExportPdfNative). Loaded at call sites that are gated by sofficeAvailable
38+
// and require.resolve() probes — keeps the legacy convert path the default
39+
// and only pulls in the in-process renderers when soffice is unconfigured.
40+
const require = createRequire(import.meta.url);
41+
3442
const fsp_writeFile = util.promisify(fs.writeFile);
3543
const fsp_unlink = util.promisify(fs.unlink);
3644

@@ -96,7 +104,6 @@ export const doExport = async (req: any, res: any, padId: string, readOnlyId: st
96104
// hand DOCX to html-to-docx and PDF to our pdfkit walker — both
97105
// pure-JS, in-process. No fallback chain: native errors surface as
98106
// 5xx so admins see real failures instead of silent shadowing.
99-
const {sofficeAvailable} = require('../utils/Settings');
100107
const sofState = sofficeAvailable();
101108
const goNative = sofState === 'no'
102109
|| (sofState === 'withoutPDF' && type === 'pdf');
@@ -105,7 +112,7 @@ export const doExport = async (req: any, res: any, padId: string, readOnlyId: st
105112
const {
106113
stripRemoteImages, extractBody, wrapLooseLines, dropEmptyBlocks,
107114
applyMonospaceToCode,
108-
} = require('../utils/ExportSanitizeHtml');
115+
} = ExportSanitizeHtml;
109116
// The HTML pipeline returns a full document (head, style, body); the
110117
// legacy soffice path renders that fine, but the in-process
111118
// converters need just the body content to avoid leaking CSS into

src/node/handler/ImportHandler.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import { Formidable } from 'formidable';
3030
import os from 'os';
3131
import * as importHtml from '../utils/ImportHtml.js';
3232
import * as importEtherpad from '../utils/ImportEtherpad.js';
33+
import * as ImportDocxNative from '../utils/ImportDocxNative.js';
34+
import * as ExportSanitizeHtml from '../utils/ExportSanitizeHtml.js';
3335
import log4js from 'log4js';
3436
import hooks from '../../static/js/pluginfw/hooks.js';
3537
import * as converterModule from '../utils/LibreOffice.js';
@@ -157,8 +159,8 @@ const performImport = async (req:any, res:any, padId:string, authorId:string) =>
157159
// through the existing setPadHTML pipeline.
158160
if (settings.soffice == null && fileEnding === '.docx') {
159161
const buf = await fs.readFile(srcFile);
160-
const {docxBufferToHtml} = require('../utils/ImportDocxNative');
161-
const {separateAdjacentHeadingBlocks} = require('../utils/ExportSanitizeHtml');
162+
const {docxBufferToHtml} = ImportDocxNative;
163+
const {separateAdjacentHeadingBlocks} = ExportSanitizeHtml;
162164
let nativeHtml: string;
163165
try {
164166
nativeHtml = await docxBufferToHtml(buf);
@@ -281,8 +283,7 @@ const performImport = async (req:any, res:any, padId:string, authorId:string) =>
281283
// Only applied to HTML imports (and converted-via-soffice
282284
// outputs, which look the same shape) -- the docx native path
283285
// above doesn't go through here.
284-
const {collapseRedundantBrAfterBlocks} =
285-
require('../utils/ExportSanitizeHtml');
286+
const {collapseRedundantBrAfterBlocks} = ExportSanitizeHtml;
286287
const cleaned = (fileIsHTML || useConverter)
287288
? collapseRedundantBrAfterBlocks(text) : text;
288289
await importHtml.setPadHTML(pad, cleaned, authorId);

src/node/hooks/express/adminsettings.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import settings, {getEpVersion, getGitCommit, reloadSettings} from '../../utils/
1111
import {getLatestVersion} from '../../utils/UpdateCheck.js';
1212
import * as padManager from '../../db/PadManager.js';
1313
import * as api from '../../db/API.js';
14+
import * as authorManager from '../../db/AuthorManager.js';
1415
import {deleteRevisions} from '../../utils/Cleanup.js';
1516

1617

@@ -308,8 +309,6 @@ export const socketio = (hookName: string, {io}: any) => {
308309
}
309310
})
310311

311-
const authorManager = require('../../db/AuthorManager');
312-
313312
// The admin author-erasure UI (PR #7667) is gated as a single
314313
// feature: when gdprAuthorErasure.enabled is false, all three
315314
// socket handlers refuse so the page is fully off by default per

src/node/hooks/express/openapi-admin.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,6 @@ export const generateAdminDefinition = (): any => ({
153153
},
154154
});
155155

156-
exports.generateAdminDefinition = generateAdminDefinition;
157-
158156
export const expressPreSession = async (
159157
_hookName: string,
160158
{app}: ArgsExpressType,
@@ -179,4 +177,3 @@ export const expressPreSession = async (
179177
});
180178
};
181179

182-
exports.expressPreSession = expressPreSession;

src/node/hooks/express/openapi.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -871,5 +871,4 @@ const generateServerForApiVersion = (apiRoot:string, req:any): {
871871
url: `${settings.ssl ? 'https' : 'http'}://${req.headers.host}${apiRoot}`,
872872
});
873873

874-
exports.generateDefinitionForVersion = generateDefinitionForVersion;
875-
exports.APIPathStyle = APIPathStyle;
874+
export {generateDefinitionForVersion, APIPathStyle};

src/node/server.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ export const start = async (): Promise<any> => {
189189
// health signal the updater's pending-verification timer is waiting for.
190190
// Wrapped in try/catch because it must never block startup on a bug here.
191191
try {
192-
// eslint-disable-next-line @typescript-eslint/no-var-requires
193-
const updater = require('./updater');
192+
const updater = await import('./updater/index.js');
194193
if (typeof updater.markBootHealthy === 'function') updater.markBootHealthy();
195194
} catch (err) {
196195
logger.debug(`markBootHealthy: ${(err as Error).message}`);

src/node/utils/ExportPdfNative.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
'use strict';
22

3+
import {createRequire} from 'node:module';
34
import {Parser} from 'htmlparser2';
45
import {PassThrough} from 'stream';
56

7+
// CJS bridge for pdfkit — it publishes a CommonJS default export
8+
// (the PDFDocument constructor) which doesn't round-trip cleanly through
9+
// ESM default-import interop under tsx.
10+
const require = createRequire(import.meta.url);
611
const PDFDocument = require('pdfkit');
712

813
interface InlineState {

src/node/utils/ImportDocxNative.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
'use strict';
22

3+
import {createRequire} from 'node:module';
4+
5+
// CJS bridge for mammoth + jszip — both publish CommonJS entries and
6+
// interact poorly with `import` default-export interop under tsx/ESM.
7+
const require = createRequire(import.meta.url);
38
const mammoth = require('mammoth');
49
const JSZip = require('jszip');
510

src/node/utils/Settings.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,9 +1343,8 @@ export const reloadSettings = () => {
13431343
if (explicit) {
13441344
settings.randomVersionString = explicit;
13451345
} else {
1346-
const pkgVersion = require('../../package.json').version as string;
13471346
settings.randomVersionString = createHash('sha256')
1348-
.update(`${pkgVersion}|${settings.gitVersion || ''}`)
1347+
.update(`${getEpVersion()}|${settings.gitVersion || ''}`)
13491348
.digest('hex')
13501349
.slice(0, 8);
13511350
}

src/static/js/pad_editbar.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,11 +517,12 @@ const padeditbar = new class {
517517
padsavedrevs.saveNow();
518518
});
519519

520-
this.registerCommand('showTimeSlider', () => {
520+
this.registerCommand('showTimeSlider', async () => {
521521
// Issue #7659: enter history in-place rather than navigating away. The
522522
// PadModeController owns the iframe lifecycle, banner, and URL hash.
523523
try {
524-
require('./pad_mode').padMode.enterHistory();
524+
const {padMode} = await import('./pad_mode.js');
525+
padMode.enterHistory();
525526
} catch (_e) {
526527
// Fallback for the unlikely case the controller failed to load.
527528
document.location = `${document.location.pathname}/timeslider`;

0 commit comments

Comments
 (0)