From 7cebd46047c43c9dcc732fe239b8792fe85fd534 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 26 Aug 2026 17:25:11 +0200 Subject: [PATCH] [monorepo] fix error code for failed ESM import (#36946) GitOrigin-RevId: 3576cba8f3b18c749b45b8f4baaaaaeb8d598c6a --- .pnp.register.mjs | 16 ++++--- package.json | 4 +- tools/migrations/package.json | 3 ++ .../test/fixtures/reportMigrationTags.mjs | 15 +++++++ tools/migrations/test/unit/adapter.test.mjs | 42 +++++++++++++++++++ 5 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 tools/migrations/test/fixtures/reportMigrationTags.mjs create mode 100644 tools/migrations/test/unit/adapter.test.mjs diff --git a/.pnp.register.mjs b/.pnp.register.mjs index 430a2293e2..1b57b8a520 100644 --- a/.pnp.register.mjs +++ b/.pnp.register.mjs @@ -30,11 +30,17 @@ registerHooks({ if (!pnp.findPackageLocator(issuer)) { return nextResolve(specifier, context) } - url = pathToFileURL( - pnp.resolveRequest(specifier, issuer, { - conditions: new Set(context.conditions), - }) - ).href + try { + url = pathToFileURL( + pnp.resolveRequest(specifier, issuer, { + conditions: new Set(context.conditions), + }) + ).href + } catch (err) { + // PnP reports a missing module with the CommonJS code. require() returned above, so this is an ESM import and node's own resolver would raise the ESM code, which is what callers of an optional import catch. + if (err.code === 'MODULE_NOT_FOUND') err.code = 'ERR_MODULE_NOT_FOUND' + throw err + } } try { // Keep the resolved path in the chain so loaders registered later can still mock it. diff --git a/package.json b/package.json index 4d173220e5..8965f9ccbd 100644 --- a/package.json +++ b/package.json @@ -191,8 +191,8 @@ "format:pug:fix": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --write --check '**/*.pug'", "format:jenkins": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --check '**/Jenkinsfile'", "format:jenkins:fix": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --write --check '**/Jenkinsfile'", - "format:monorepo-check": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --check '**/Jenkinsfile' 'google-ops/packer/jenkins/vm_config/init.groovy.d/*.groovy' '**/*.md' '**/docker-compose.yml' '**/docker-compose.*.yml' '.agents/**/*.js' '.agents/**/*.md' '.github/**/*.md' .pnp.register.mjs", - "format:monorepo-check:fix": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --write --check '**/Jenkinsfile' 'google-ops/packer/jenkins/vm_config/init.groovy.d/*.groovy' '**/*.md' '**/docker-compose.yml' '**/docker-compose.*.yml' '.agents/**/*.js' '.agents/**/*.md' '.github/**/*.md' .pnp.register.mjs", + "format:monorepo-check": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --check '**/Jenkinsfile' 'google-ops/packer/jenkins/vm_config/init.groovy.d/*.groovy' '**/*.md' '**/docker-compose.yml' '**/docker-compose.*.yml' '.agents/**/*.js' '.agents/**/*.md' '.github/**/*.md' 'tools/monorepo-check/**/*.mjs' .pnp.register.mjs", + "format:monorepo-check:fix": "prettier --cache --cache-location ./.cache/prettier/.prettier-cache --write --check '**/Jenkinsfile' 'google-ops/packer/jenkins/vm_config/init.groovy.d/*.groovy' '**/*.md' '**/docker-compose.yml' '**/docker-compose.*.yml' '.agents/**/*.js' '.agents/**/*.md' '.github/**/*.md' 'tools/monorepo-check/**/*.mjs' .pnp.register.mjs", "lint": "eslint --cache --cache-location ./.cache/eslint/ --max-warnings 0", "lint:fix": "eslint --cache --cache-location ./.cache/eslint/ --fix" }, diff --git a/tools/migrations/package.json b/tools/migrations/package.json index 436b2b1aa3..5d50065bba 100644 --- a/tools/migrations/package.json +++ b/tools/migrations/package.json @@ -2,6 +2,9 @@ "name": "@overleaf/migrations", "scripts": { "migrations": "MONGO_SOCKET_TIMEOUT=0 east --es-modules", + "test": "yarn run lint && yarn run test:unit", + "test:unit": "node --test 'test/unit/*.test.mjs'", + "test:ci": "yarn run test:unit", "lint": "eslint --cache --cache-location ../../.cache/eslint/ --ext .cjs,.js,.jsx,.mjs,.ts --max-warnings 0 .", "lint:fix": "eslint --cache --cache-location ../../.cache/eslint/ --fix --ext .cjs,.js,.jsx,.mjs,.ts ." }, diff --git a/tools/migrations/test/fixtures/reportMigrationTags.mjs b/tools/migrations/test/fixtures/reportMigrationTags.mjs new file mode 100644 index 0000000000..f50ca60940 --- /dev/null +++ b/tools/migrations/test/fixtures/reportMigrationTags.mjs @@ -0,0 +1,15 @@ +import Path from 'node:path' +import Adapter from '../../lib/adapter.mjs' + +// The adapter aborts unless a tag is passed on argv, a CLI-only guard. +process.env.SKIP_TAG_CHECK = '1' + +const adapter = new Adapter({ + dir: Path.resolve(import.meta.dirname, '../..'), + migrationExtension: 'mjs', +}) + +console.log(JSON.stringify(await adapter._getMigrationTags(process.argv[2]))) + +// lib/mongodb.mjs connects when imported and nothing here needs the connection. +process.exit(0) diff --git a/tools/migrations/test/unit/adapter.test.mjs b/tools/migrations/test/unit/adapter.test.mjs new file mode 100644 index 0000000000..aa4b47eacf --- /dev/null +++ b/tools/migrations/test/unit/adapter.test.mjs @@ -0,0 +1,42 @@ +import assert from 'node:assert/strict' +import Path from 'node:path' +import { describe, it } from 'node:test' +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' + +const execFileAsync = promisify(execFile) + +const MIGRATIONS_DIR = Path.resolve(import.meta.dirname, '../..') +const FIXTURE = Path.join( + MIGRATIONS_DIR, + 'test', + 'fixtures', + 'reportMigrationTags.mjs' +) + +// The adapter opens a Mongo connection when imported, so the tags are read in a subprocess that exits as soon as it has them. +async function readMigrationTags(name) { + const { stdout } = await execFileAsync(process.execPath, [FIXTURE, name], { + cwd: MIGRATIONS_DIR, + }) + // @overleaf/settings logs to stdout when it loads, so the tags are on the last line. + return JSON.parse(stdout.trim().split('\n').pop()) +} + +describe('Adapter', function () { + describe('_getMigrationTags', function () { + it('reads the tags a migration declares', async function () { + assert.deepEqual( + await readMigrationTags('20190720165251_create_migrations'), + ['server-ce', 'server-pro', 'saas'] + ) + }) + + it('reads no tags for a migration that is recorded as executed but no longer on disk', async function () { + assert.deepEqual( + await readMigrationTags('20190730093801_script_example'), + [] + ) + }) + }) +})