The first CI run failed on `node:test`, in every test file, reported as the module boundary being broken. It was not: `builtinModules` omits `test` on Node 20 (CI) and includes it on Node 24 (local), so a list rebuilt from it disagrees with itself across versions. Use `isBuiltin`, which is Node's own answer, and treat the `node:` prefix as sufficient on its own -- a prefixed specifier can never resolve to a package, whatever the running version enumerates. Test covers both forms. Also corrects this file's header: the client half's guard is no longer `external` (it never worked), it is the Vite build's resolution-time check plus checkExternals.js on the built chunk. Co-Authored-By: Claude <noreply@anthropic.com>
150 lines
6.0 KiB
JavaScript
150 lines
6.0 KiB
JavaScript
// The boundary check, checked.
|
|
//
|
|
// `scripts/checkImports.js` is the acceptance test for the whole module contract
|
|
// (MODULE_API.md §5.1), and a check that has never been shown to fail is a check
|
|
// nobody knows the state of. These point it at fixtures that break each rule and
|
|
// assert it says so — and at prose that merely *describes* breaking them, which
|
|
// is what it got wrong the first time it was run.
|
|
//
|
|
// **Every fixture is a template literal, and that is load-bearing.** The scanner
|
|
// reads the files in this directory too, so an ordinary quoted string holding
|
|
// `require('../../x')` would make this file fail the very check it is testing.
|
|
// Templates are blanked by the stripper for exactly this class of text: source
|
|
// being composed as data is not source being imported.
|
|
|
|
const test = require('node:test')
|
|
const assert = require('node:assert')
|
|
const fs = require('node:fs')
|
|
const os = require('node:os')
|
|
const path = require('node:path')
|
|
|
|
const { scan, stripCommentsAndTemplates, SERVER_ROOT, MODULE_ROOT } = require('../scripts/checkImports')
|
|
|
|
/** Write `files` into a throwaway module tree and scan it. */
|
|
function scanFixture(files, { dev = new Set() } = {}) {
|
|
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'module-uo-'))
|
|
const src = path.join(root, 'server')
|
|
for (const [name, source] of Object.entries(files)) {
|
|
const file = path.join(src, name)
|
|
fs.mkdirSync(path.dirname(file), { recursive: true })
|
|
fs.writeFileSync(file, source)
|
|
}
|
|
try {
|
|
return scan(src, root, { shipped: (f) => !f.startsWith(path.join(src, 'test') + path.sep), dev })
|
|
} finally {
|
|
fs.rmSync(root, { recursive: true, force: true })
|
|
}
|
|
}
|
|
|
|
test('the real server half is clean', () => {
|
|
assert.deepStrictEqual(scan(SERVER_ROOT, MODULE_ROOT), [])
|
|
})
|
|
|
|
test('catches a relative path that escapes the module root', () => {
|
|
const found = scanFixture({ 'a.js': `require('../../server/src/utils/db')` })
|
|
assert.strictEqual(found.length, 1)
|
|
assert.strictEqual(found[0].why, 'escapes the module root')
|
|
})
|
|
|
|
test('allows a relative path that stays inside it, however deep', () => {
|
|
assert.deepStrictEqual(
|
|
scanFixture({ 'deep/nested/a.js': `require('../../../module.json')` }),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('catches an absolute path', () => {
|
|
const found = scanFixture({ 'a.js': `require('/etc/passwd')` })
|
|
assert.strictEqual(found[0].why, 'absolute path')
|
|
})
|
|
|
|
test('catches a bare specifier in shipped code, even a devDependency', () => {
|
|
// The rule that makes the boundary real: express arrives on ctx. A shipped
|
|
// file requiring it would fail on a real install, because a module lives
|
|
// outside core's server/ and never reaches core's node_modules.
|
|
const found = scanFixture({ 'a.js': `const express = require('express')` }, { dev: new Set(['express']) })
|
|
assert.strictEqual(found.length, 1)
|
|
assert.match(found[0].why, /should this come from ctx/)
|
|
})
|
|
|
|
test('allows a devDependency in test code, which never runs inside core', () => {
|
|
assert.deepStrictEqual(
|
|
scanFixture({ 'test/a.js': `const express = require('express')` }, { dev: new Set(['express']) }),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('allows node builtins anywhere, with or without the node: prefix', () => {
|
|
assert.deepStrictEqual(
|
|
scanFixture({ 'a.js': `require('path'); require('node:fs'); import crypto from 'node:crypto'` }),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('allows node:test, which older Node versions omit from builtinModules', () => {
|
|
// The first CI run failed on exactly this and on nothing else: `builtinModules`
|
|
// omits `test` on Node 20 and includes it on Node 24, so every test file in
|
|
// this suite was reported as breaking the module boundary. The check asks
|
|
// Node (`isBuiltin`) rather than rebuilding the list, and treats the `node:`
|
|
// prefix as sufficient on its own — a prefixed specifier can never resolve to
|
|
// a package, whatever the running version enumerates.
|
|
assert.deepStrictEqual(
|
|
scanFixture({ 'a.js': `require('node:test'); require('node:test/reporters')` }),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('catches ESM and dynamic forms, not only require()', () => {
|
|
const found = scanFixture({
|
|
'a.js': [`import db from '../../core/db.js'`, `const x = await import('../../core/other.js')`].join('\n'),
|
|
})
|
|
assert.strictEqual(found.length, 2)
|
|
})
|
|
|
|
test('ignores a violation that is only DESCRIBED in a comment', () => {
|
|
// The first run of this check failed on its own documentation, and on
|
|
// index.js's comment explaining why the module must never require('express').
|
|
// Prose about the rule must not trip the rule.
|
|
assert.deepStrictEqual(
|
|
scanFixture({
|
|
'a.js': [
|
|
`// Never write require("../../server/src/utils/db") - it escapes the module root.`,
|
|
`/* Nor import express from "express": core hands it over on ctx. */`,
|
|
`const path = require('path')`,
|
|
].join('\n'),
|
|
}),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('ignores a specifier-shaped string inside a template literal', () => {
|
|
assert.deepStrictEqual(
|
|
scanFixture({ 'a.js': ['const sql = ', '`SELECT 1 -- require("../../x")`'].join('') }),
|
|
[],
|
|
)
|
|
})
|
|
|
|
test('a comment opener inside a string does not swallow the rest of the file', () => {
|
|
// The reason this is a character walk and not a regexp: a URL in a string
|
|
// contains `//`, and treating that as a comment would blank everything after
|
|
// it — turning the check into one that silently passes.
|
|
const found = scanFixture({
|
|
'a.js': [`const url = 'https://example.com/x'`, `require('../../escaped')`].join('\n'),
|
|
})
|
|
assert.strictEqual(found.length, 1, 'the specifier after a URL string was missed')
|
|
})
|
|
|
|
test('a quote inside a comment does not swallow the rest of the file', () => {
|
|
const found = scanFixture({
|
|
'a.js': [`// don't do this`, `require('../../escaped')`].join('\n'),
|
|
})
|
|
assert.strictEqual(found.length, 1)
|
|
})
|
|
|
|
test('stripping preserves line numbers', () => {
|
|
// Blanked rather than removed, so anything that later reports a line still
|
|
// reports the right one.
|
|
const src = ['/* a', 'b', 'c */', `require("x")`, ''].join('\n')
|
|
assert.strictEqual(stripCommentsAndTemplates(src).split('\n').length, src.split('\n').length)
|
|
})
|