deckhearth/.convoys/lint-against-cjs-in-esm-scripts.md
varutasu 13d62108ce
chore(lint): forbid require() in scripts/** under "type": "module" (#29)
Add an ESLint no-restricted-syntax rule scoped to scripts/**/*.js
that flags any CallExpression with callee name `require`. The
recurring bug pattern: helper scripts under scripts/ that use CJS
require() throw `ReferenceError: require is not defined` on Node
22.x because package.json has had "type": "module" since
bump-next-js. The bug has bitten twice in two convoys — once in
drop-public-setup Brief 2 (setup-neon-db.js, commit b63b509) and
again in fix-reset-db-script (reset-db.js, PR #25 squash 3ab9bf8).
Both were caught at first run, not at lint time. This rule would
have caught both at PR time.

Rule shape: a second flat-config block at the end of
eslint.config.mjs (NOT in the root rules block) targeting only
scripts/**/*.js. The error message points at
.convoys/fix-reset-db-script.md so the next agent who trips it
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 and stays excluded.

Blast-radius rationale (scripts/** only, not all .js at repo root):
matches the actual observed bug surface. pages/api/** is already
correctly ESM-imported throughout (verified across add-rate-limiting,
cors-tighten, and the add-route skill). The config files
(postcss.config.js, tailwind.config.js, next.config.js) intentionally
use CJS-style exports that the next-config base rules already handle
correctly. A repo-wide ban would produce zero true positives outside
scripts/** today and would require explicit allowlist for every
config file — strictly more code, more maintenance, zero benefit.

Convoy file: .convoys/lint-against-cjs-in-esm-scripts.md (P3 polish,
parent-owned, no architect — preventative one-line rule following
two proven bug recurrences).

Verification:
- node --check eslint.config.mjs: exit 0
- npm run lint: 128 problems (81 errors, 47 warnings) — baseline
  preserved verbatim, zero new false positives in current tree
- Negative test: prepended `const x = require('fs');` to
  scripts/reset-db.js, ran npm run lint, observed exit 1 with
  129 problems and the rule firing at line 20:11 with the
  documented message, then reverted to 128 problems clean
- npm run test:run: 21/21 pass (no test surface touched)
- Grep: 0 require( occurrences in scripts/**/*.js (current tree
  is clean; rule starts with zero positives to silence on day 1)

Surfaces no new follow-up — this convoy IS the follow-up surfaced
by fix-reset-db-script.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-26 22:53:31 -05:00

7.9 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: open 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

(Stub — doc-writer to fill in post-merge with squash commit SHA, PR URL, observed lint baseline before/after, CI gate results, and any deviations from the planned shape.)