deckhearth/.convoys/lint-against-cjs-in-esm-scripts.md
varutasu ad0e254324
chore(convoys): mark shipped convoys and refresh ship-readiness (#66)
Close stale convoy frontmatter for merged scanner, lint, and hygiene work;
record P1 #11.5 and queued follow-ups as RESOLVED with PR references.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-02 11:01:27 -05:00

12 KiB


name: lint-against-cjs-in-esm-scripts classification: hygiene success_metric: future helper scripts that re-introduce CJS require() calls under package.json "type": "module" fail at lint time, not at first execution status: shipped created: 2026-05-26

lint-against-cjs-in-esm-scripts (P3 polish — parent-owned)

Priority: P3 polish (one-line ESLint rule; no architect required) Convoy owner: parent Opened: 2026-05-26

Background — the recurring bug pattern

Since bump-next-js flipped package.json to "type": "module", any helper script under scripts/ that uses CJS require() throws ReferenceError: require is not defined on Node 22.x at first run. The same bug has now bitten the repo twice in two convoys:

  1. drop-public-setup Brief 2 (commit b63b509, 2026-05-23): scripts/setup-neon-db.js was still CJS post-bump-next-js; npm run setup-db was silently broken until Brief 2 swept it to ESM imports. The convoy retro called this out as "the seed script silently stopped executing after bump-next-js."
  2. fix-reset-db-script Brief 1 (commit 3ab9bf8, PR #25, 2026-05-26): three require() calls in scripts/reset-db.js (lines 10, 12, 142) — same bug, same blast radius (npm run reset-db throws ReferenceError), same fix shape (verbatim mirror of post-drop-public-setup setup-neon-db.js).

Both bugs were caught at first run, not at lint time. A small ESLint rule scoped to scripts/**/*.js would have caught both at PR time and is cheap insurance against a third recurrence.

Design decision — scripts/** only (NOT all .js)

Two reasonable scopes:

  • scripts/**/*.js (chosen): matches the actual blast radius — every observed instance of the bug has been in a helper script. Per-file-block override in eslint.config.mjs via a second flat- config entry. Zero impact on pages/api/** (already correctly ESM-imported throughout) and zero impact on the root *.config.js files (which are intentionally CJS-shaped and which the next-config base rules already handle correctly).
  • All .js files at repo root (rejected): broader-than-necessary blast radius. pages/api/** already uses ESM import everywhere (add-rate-limiting, cors-tighten, and add-route skill all verified this in the last three months). A repo-wide ban would produce zero true positives outside scripts/** today and would risk breaking config-file shapes that legitimately use CJS (postcss.config.js, tailwind.config.js are flagged by import/no-anonymous-default-export today but read CJS-style exports under the hood — see also Gotcha #9 + #10).

The scoped rule is a 7-line flat-config block; the broader rule would require explicit allowlist for every config file, which is strictly more code and more maintenance.

The fix

Add a new flat-config block at the end of eslint.config.mjs (after the existing globalIgnores(...) call, NOT inside the root rules block) targeting only scripts/**/*.js:

{
  files: ['scripts/**/*.js'],
  rules: {
    'no-restricted-syntax': ['error', {
      selector: 'CallExpression[callee.name="require"]',
      message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.',
    }],
  },
},

The error message points at .convoys/fix-reset-db-script.md so that the next agent / contributor who triggers the rule gets a 1-click path to the exemplar fix (ESM top-level imports for dotenv, neon, bcrypt) instead of having to re-derive it.

scripts/migrations/** is already in globalIgnores (from pick-a-name Brief 2's migration script) and stays ignored — the rule does not fire there even though the migration script is ESM and correctly uses import (no need to re-lint files already excluded).

The rule fires on CallExpression[callee.name="require"] — the AST shape of a plain require('foo') call. It does NOT fire on createRequire(import.meta.url) patterns (which use Module.createRequire) should one ever be needed; the AST callee is createRequire, not require. If a future helper script legitimately needs CJS interop, the right path is await import('foo') (ESM dynamic import) — the rule will not block that either.

Verification plan

  1. node --check eslint.config.mjs → exit 0 (config parses).
  2. npm run lint → exit 1 with 128 problems (81 errors, 47 warnings) — verbatim match of the pre-convoy baseline (no regression, no new false positives in the current tree).
  3. Negative test (apply, run, revert): prepend const x = require('fs'); to scripts/reset-db.js, run npm run lint, confirm exit 1 with the new rule firing at the expected line/column and the documented message, then revert.
  4. npm run test:run → 21/21 pass (no test surface touched; runs only to confirm vitest is still green).
  5. rg "require\(" scripts/ --type js → 0 hits (sanity check confirming the current tree is clean and the rule has zero positives to silence on day 1).

Risks

  • False positives if anyone legitimately needs require() in scripts/**. None today (verified by step 5 — zero require( hits across all helper scripts in the current tree after PR #25 and the drop-public-setup B2 sweep). If a future script legitimately needs CJS interop (e.g. a dependency that only exports CJS without an ESM wrapper), the fix is await import('foo') — ESM dynamic import works in any ESM script and is not flagged by the rule. If that's somehow not viable, the escape hatch is a per-line // eslint-disable-next-line no-restricted-syntax with a comment explaining why ESM doesn't work; lint baseline tracking will catch the disable directive in review.
  • Rule scope drift. If someone adds a new top-level scripts directory (tools/, cli/, etc.) the rule won't fire there. Low risk — this repo has consolidated on scripts/ since inception and there's no signal of a second scripts directory being added. Tracked here so the next refactor that reshapes the helper-script layout knows to extend the files: glob.
  • ESLint v10 bump. When bump-eslint-10 lands (currently upstream-blocked per Gotcha #10), re-verify this rule's selector syntax against the v10 AST behavior. no-restricted-syntax is a stable core rule going back to ESLint v1; no v10 deprecation is expected, but the smoke check is cheap.

Acceptance criteria

  • node --check eslint.config.mjs exit 0
  • npm run lint exit 1 with 128 problems (baseline preserved)
  • npm run test:run 21/21 pass
  • Negative test passes (rule fires on synthetic require() insertion, reverts cleanly to 128 problems after the synthetic edit is removed)
  • Grep: 0 require( occurrences in scripts/**/*.js (current tree is clean — rule starts with zero positives to silence)

Out of scope

  • Sweeping any other scripts/** file — current tree is clean (verified by the grep step above). The rule is preventative, not retroactive.
  • Broadening the rule to all .js files at repo root — see § Design decision; pages/api/** is already correctly ESM and the config files (postcss.config.js, tailwind.config.js, next.config.js) intentionally use CJS-style exports that the next-config base rules handle correctly.
  • Bumping any deps (ESLint stays at v9 per Gotcha #10; no typescript-eslint interaction since the rule is a core rule).
  • The purge-weak-creds-from-helpers follow-up (the scripts/create-test-users.js portion remains queued; this convoy only adds the lint rule, not the weak-creds sweep).

Owns

Parent (single-file ESLint config edit; no architect or implementer subagent required — proven-pattern follow-up to PR #25).

As-shipped

Single squash commit 13d6210 (PR #29, merged 2026-05-27T03:53:31Z UTC / local 2026-05-26). Parent-owned end-to-end per the convoy spec — no architect, no implementer subagent dispatched. Single-file ESLint config edit following a proven-pattern follow-up shape; no mid-execution surprises.

Diff: 2 files, +185 / -0. eslint.config.mjs (the 7-line flat-config block added after the existing globalIgnores(...) call, plus a leading comment block referencing the two motivating bugs) + .convoys/lint-against-cjs-in-esm-scripts.md (the planning document, committed atomically with the rule).

The change shipped exactly as designed. A new flat-config block appended to eslint.config.mjs:

{
  files: ['scripts/**/*.js'],
  rules: {
    'no-restricted-syntax': ['error', {
      selector: 'CallExpression[callee.name="require"]',
      message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.',
    }],
  },
},

The error message points at .convoys/fix-reset-db-script.md so a future contributor / agent who trips the rule gets a 1-click path to the exemplar ESM fix shape (top-level import dotenv from 'dotenv', import { neon } from '@neondatabase/serverless', import bcrypt from 'bcryptjs') instead of having to re-derive it.

scripts/migrations/** is already in globalIgnores (from pick-a-name Brief 2's migration script) and stays ignored — the rule does not fire there even though the migration script is ESM and correctly uses import. No need to re-lint files already excluded.

Verification (all gates green at merge):

  • node --check eslint.config.mjs → exit 0 (config parses)
  • npm run lint → exit 1 with 125 problems (post-PR-#31 baseline preserved; zero new false positives in the current tree because both motivating bugs were already fixed: setup-neon-db.js swept by drop-public-setup B2 and reset-db.js swept by fix-reset-db-script PR #25)
  • Negative test verified (apply, run, revert): prepending const x = require('fs'); to scripts/reset-db.js fired the rule at the expected line/column with the documented message; reverting returned to a clean 125-problem lint. The rule starts with zero positives to silence on day 1, which is the documented success shape — preventative, not retroactive.
  • npm run test:run → 21/21 pass (no test surface touched; verification only)
  • rg "require\(" scripts/ --type js → 0 hits (sanity check confirming the current tree is clean)
  • CI on PR #29: Lint ✓ | Vitest 21/21 ✓ | Playwright smoke 3/3 ✓ | forbidden-endpoints ✓ | forbidden-cors-headers ✓ | Vercel preview deploy ✓ | Aggregate gate ✓
  • Screenshot diff: not triggered (PR #29 touches only eslint.config.mjs + this convoy file — neither matches the visual-diff paths: filter)

Both motivating bugs WOULD have been caught at lint time. Both drop-public-setup Brief 2's pre-fix scripts/setup-neon-db.js (three require() calls at lines 1-3 pre-fix) and fix-reset-db-script's pre-fix scripts/reset-db.js (three require() calls at lines 10, 12, 142 pre-fix) would have triggered the new rule at PR time instead of throwing ReferenceError: require is not defined at first execution. This is the exact "would have caught both bugs" shape that motivated the queue entry in .convoys/ship-readiness.md.

Operator action required going forward: none. The rule is self-defending; no env vars, no secrets, no infra changes. Future helper scripts under scripts/** that re-introduce CJS require() fail at lint time with the documented message + the exemplar pointer to .convoys/fix-reset-db-script.md.

Spec deviation: none. The flat-config block shipped exactly as the convoy file's § The fix described it. Selector, message, scope (scripts/**/*.js only — NOT all .js), and globalIgnores interaction all match the spec verbatim.

No follow-up surfaced. Pairs naturally with the queued lint-against-lib-database follow-up from .convoys/single-sql-client.md (both are static-source guards added to eslint.config.mjs); they could fold into a harden-eslint-static-guards convoy if more such guards accumulate.