diff --git a/.config/rollup.dist.config.mjs b/.config/rollup.dist.config.mjs index 52eab0bec..154474889 100644 --- a/.config/rollup.dist.config.mjs +++ b/.config/rollup.dist.config.mjs @@ -74,6 +74,22 @@ const SOCKET_SECURITY_REGISTRY = '@socketsecurity/registry' const UTILS = 'utils' const VENDOR = 'vendor' +// A fresh regexp per call: socketModifyPlugin advances lastIndex, so a shared +// instance would make chunks skip each other's matches. +function newBareBlessedRequireRegExp() { + return /(?<=require[$\w]*(?:\.resolve)?\(["'])blessed(?=(?:\/[^"']+)?["']\))/g +} + +// '.' rather than '' keeps a require from a file sitting in blessed's own root +// from becoming an absolute-looking specifier. +function relativeBlessedPath(filepath) { + return ( + normalizePath( + path.relative(path.dirname(filepath), constants.blessedPath), + ) || '.' + ) +} + async function copyInitGradle() { const filepath = path.join(constants.srcPath, 'commands/manifest/init.gradle') const destPath = path.join(constants.distPath, 'init.gradle') @@ -197,22 +213,26 @@ async function copyExternalPackages() { await removeEmptyDirs(thePath) }), ) - // Rewire 'blessed' inside 'blessed-contrib'. + // Rewire 'blessed' inside 'blessed-contrib', and inside blessed's own vendor + // files, which reach for it by bare name too. await Promise.all( - ( - await fastGlob.glob(['**/*.js'], { + [blessedPath, blessedContribPath].map(async cwd => { + const filepaths = await fastGlob.glob(['**/*.js'], { absolute: true, - cwd: blessedContribPath, + cwd, ignore: [NODE_MODULES_GLOB_RECURSIVE], }) - ).map(async p => { - const relPath = path.relative(path.dirname(p), blessedPath) - const content = await fs.readFile(p, 'utf8') - const modded = content.replace( - /(?<=require\(["'])blessed(?=(?:\/[^"']+)?["']\))/g, - () => relPath, + await Promise.all( + filepaths.map(async p => { + const relPath = relativeBlessedPath(p) + const content = await fs.readFile(p, 'utf8') + const modded = content.replace( + newBareBlessedRequireRegExp(), + () => relPath, + ) + await fs.writeFile(p, modded, 'utf8') + }), ) - await fs.writeFile(p, modded, 'utf8') }), ) } @@ -563,6 +583,11 @@ export default async () => { ) }, plugins: [ + // Runs after copyExternalPackages() and overwrites what it rewired. + socketModifyPlugin({ + find: newBareBlessedRequireRegExp(), + replace: () => relativeBlessedPath(path.join(rootPath, relPath)), + }), nodeResolve({ exportConditions: ['node'], extensions: ['.mjs', '.js', '.json'], diff --git a/test/external-bare-requires.test.mts b/test/external-bare-requires.test.mts new file mode 100644 index 000000000..40fc951eb --- /dev/null +++ b/test/external-bare-requires.test.mts @@ -0,0 +1,91 @@ +/** + * Guards that nothing under external/ reaches for a bundled package by bare + * name. Node resolves bare specifiers only through node_modules directories, + * and external/ is not one, so such a require throws "Cannot find module" in + * the published package even though the file ships correctly on disk. + */ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +import { describe, expect, it } from 'vitest' + +const rootPath = path.join(path.dirname(fileURLToPath(import.meta.url)), '..') +const externalPath = path.join(rootPath, 'external') + +// Mirrors EXTERNAL_PACKAGES in .config/rollup.base.config.mjs. Scoped to what +// the build vendors into external/, because unbundled optional peers reached by +// bare name there (blessed's pty.js/term.js terminal widget, node-gyp under +// @socketsecurity/registry) are a separate, longstanding question. +const bundledNames = new Set([ + '@socketsecurity/registry', + 'blessed', + 'blessed-contrib', +]) + +const bareRequireRegExp = + /require[$\w]*(?:\.resolve)?\(\s*['"]([^'"]+)['"]\s*\)/g + +function findScripts(dirPath: string): string[] { + const scriptPaths: string[] = [] + for (const entry of readdirSync(dirPath, { withFileTypes: true })) { + const entryPath = path.join(dirPath, entry.name) + if (entry.isDirectory()) { + scriptPaths.push(...findScripts(entryPath)) + } else if (entry.name.endsWith('.js')) { + scriptPaths.push(entryPath) + } + } + return scriptPaths.sort() +} + +function packageNameFromSpecifier(specifier: string): string | undefined { + if ( + !specifier || + specifier.startsWith('.') || + specifier.startsWith('#') || + specifier.startsWith('node:') || + path.isAbsolute(specifier) + ) { + return undefined + } + const segments = specifier.split('/') + return specifier.startsWith('@') + ? segments.slice(0, 2).join('/') + : segments[0] +} + +describe('external bare requires', () => { + it('never name a bundled package', () => { + if (!existsSync(externalPath)) { + throw new Error( + `Missing build output at ${externalPath}.\n` + + `→ This test checks what ships, so it needs a built external/.\n` + + `→ Run: pnpm build:dist:src`, + ) + } + const scriptPaths = findScripts(externalPath) + expect(scriptPaths.length).toBeGreaterThan(0) + + const findings: string[] = [] + for (const scriptPath of scriptPaths) { + const relPath = path.relative(rootPath, scriptPath).replace(/\\/g, '/') + const source = readFileSync(scriptPath, 'utf8') + bareRequireRegExp.lastIndex = 0 + let match + while ((match = bareRequireRegExp.exec(source)) !== null) { + const specifier = match[1]! + const pkgName = packageNameFromSpecifier(specifier) + if (!pkgName || !bundledNames.has(pkgName)) { + continue + } + const finding = `${relPath} requires "${specifier}"` + if (!findings.includes(finding)) { + findings.push(finding) + } + } + } + + expect(findings).toEqual([]) + }) +})