chore(lint): forbid require() in scripts/** under "type": "module" #29

Merged
varutasu merged 1 commit from convoy/lint-against-cjs-in-esm-scripts into main 2026-05-26 23:53:31 -04:00
varutasu commented 2026-05-26 23:50:46 -04:00 (Migrated from github.com)

Summary

Adds an ESLint no-restricted-syntax rule scoped to scripts/**/*.js that flags any CallExpression with callee name require. The rule is preventative — the current tree is already clean (0 hits) — and exists to stop the recurring CJS-in-ESM bug from re-shipping in a future helper script.

Why

Since bump-next-js flipped package.json to "type": "module", any helper script that uses CJS require() throws ReferenceError: require is not defined on Node 22.x. This bug has now shipped twice and been caught at first-run-on-the-developer's-machine both times rather than at PR time:

  1. drop-public-setup Brief 2 (commit b63b509) — scripts/setup-neon-db.js was silently broken post-bump-next-js until B2 swept it to ESM imports.
  2. fix-reset-db-script (PR #25, squash 3ab9bf8) — three require() calls in scripts/reset-db.js (lines 10, 12, 142); npm run reset-db threw ReferenceError on Node 22.x until PR #25 mirrored the post-DPS shape.

Both bugs were lint-clean before they shipped. This rule would have failed lint on both PRs and saved the round-trip.

The rule

One new flat-config block at the end of eslint.config.mjs (NOT in the root rules block):

{
  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 reviewers / agents at the exemplar fix (post-DPS setup-neon-db.js shape: ESM top-level imports for dotenv, neon, bcrypt) instead of forcing them to re-derive it.

Blast-radius rationale: scripts/** only, NOT all .js at repo root. pages/api/** is already correctly ESM-imported throughout (verified across add-rate-limiting, cors-tighten, and the add-route skill). The root config files (postcss.config.js, tailwind.config.js, next.config.js) intentionally use CJS-style exports that the next-config base rules handle correctly. A repo-wide ban would produce zero true positives outside scripts/** today and would force explicit allowlist entries for every config file — strictly more code, more maintenance, zero benefit. See .convoys/lint-against-cjs-in-esm-scripts.md § Design decision.

scripts/migrations/** is already in globalIgnores from pick-a-name B2 and stays excluded; no double-handling needed.

Verification

  • node --check eslint.config.mjs → exit 0

  • npm run lint128 problems (81 errors, 47 warnings) — verbatim baseline preservation, zero new false positives in the current tree

  • Negative test (apply, run, revert): 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:

    scripts/reset-db.js
      20:11  error  Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md  no-restricted-syntax
    

    Then reverted the synthetic edit — clean tree, back to 128 problems.

  • npm run test:run → 21/21 pass (no test surface touched)

  • Sanity grep: rg "require\(" scripts/ --type js → 0 hits (current tree is clean; rule starts with zero positives to silence)

Test plan

  • node --check eslint.config.mjs passes locally
  • npm run lint baseline preserved at 128 problems locally
  • Negative test (synthetic require() insert → rule fires → revert) passes locally
  • npm run test:run 21/21 pass locally
  • CI: Lint job stays green on this PR
  • CI: Vitest stays 21/21 green
  • CI: forbidden-endpoints + forbidden-cors-headers gates stay green
  • CI: Playwright smoke stays 3/3 green
  • CI: Screenshot diff NOT triggered (config-only PR — paths: filter excludes eslint.config.mjs and .convoys/**)

Follow-ups

None new — this convoy IS the follow-up surfaced by fix-reset-db-script (PR #25). The purge-weak-creds-from-helpers follow-up (the scripts/create-test-users.js portion) remains queued independently and is out of scope here.

Made with Cursor

## Summary Adds an ESLint `no-restricted-syntax` rule scoped to `scripts/**/*.js` that flags any CallExpression with callee name `require`. The rule is preventative — the current tree is already clean (0 hits) — and exists to stop the recurring CJS-in-ESM bug from re-shipping in a future helper script. ## Why Since `bump-next-js` flipped `package.json` to `"type": "module"`, any helper script that uses CJS `require()` throws `ReferenceError: require is not defined` on Node 22.x. This bug has now shipped twice and been caught at first-run-on-the-developer's-machine both times rather than at PR time: 1. **`drop-public-setup` Brief 2** (commit `b63b509`) — `scripts/setup-neon-db.js` was silently broken post-`bump-next-js` until B2 swept it to ESM imports. 2. **`fix-reset-db-script`** (PR #25, squash `3ab9bf8`) — three `require()` calls in `scripts/reset-db.js` (lines 10, 12, 142); `npm run reset-db` threw `ReferenceError` on Node 22.x until PR #25 mirrored the post-DPS shape. Both bugs were lint-clean before they shipped. This rule would have failed lint on both PRs and saved the round-trip. ## The rule One new flat-config block at the end of `eslint.config.mjs` (NOT in the root rules block): ```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 reviewers / agents at the exemplar fix (post-DPS `setup-neon-db.js` shape: ESM top-level imports for `dotenv`, `neon`, `bcrypt`) instead of forcing them to re-derive it. **Blast-radius rationale: `scripts/**` only, NOT all `.js` at repo root.** `pages/api/**` is already correctly ESM-imported throughout (verified across `add-rate-limiting`, `cors-tighten`, and the `add-route` skill). The root config files (`postcss.config.js`, `tailwind.config.js`, `next.config.js`) intentionally use CJS-style exports that the next-config base rules handle correctly. A repo-wide ban would produce zero true positives outside `scripts/**` today and would force explicit allowlist entries for every config file — strictly more code, more maintenance, zero benefit. See `.convoys/lint-against-cjs-in-esm-scripts.md` § Design decision. `scripts/migrations/**` is already in `globalIgnores` from `pick-a-name` B2 and stays excluded; no double-handling needed. ## Verification - `node --check eslint.config.mjs` → exit 0 - `npm run lint` → **128 problems (81 errors, 47 warnings)** — verbatim baseline preservation, zero new false positives in the current tree - **Negative test (apply, run, revert):** 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: ``` scripts/reset-db.js 20:11 error Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md no-restricted-syntax ``` Then reverted the synthetic edit — clean tree, back to 128 problems. - `npm run test:run` → 21/21 pass (no test surface touched) - Sanity grep: `rg "require\(" scripts/ --type js` → 0 hits (current tree is clean; rule starts with zero positives to silence) ## Test plan - [x] `node --check eslint.config.mjs` passes locally - [x] `npm run lint` baseline preserved at 128 problems locally - [x] Negative test (synthetic `require()` insert → rule fires → revert) passes locally - [x] `npm run test:run` 21/21 pass locally - [ ] CI: Lint job stays green on this PR - [ ] CI: Vitest stays 21/21 green - [ ] CI: `forbidden-endpoints` + `forbidden-cors-headers` gates stay green - [ ] CI: `Playwright smoke` stays 3/3 green - [ ] CI: `Screenshot diff` NOT triggered (config-only PR — `paths:` filter excludes `eslint.config.mjs` and `.convoys/**`) ## Follow-ups None new — this convoy IS the follow-up surfaced by `fix-reset-db-script` (PR #25). The `purge-weak-creds-from-helpers` follow-up (the `scripts/create-test-users.js` portion) remains queued independently and is out of scope here. Made with [Cursor](https://cursor.com)
vercel[bot] commented 2026-05-26 23:50:51 -04:00 (Migrated from github.com)

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tcg-vault Ready Ready Preview, Comment May 27, 2026 3:50am

Request Review

[vc]: #krZje8T44pavd6tR+CACRN8A7AmM5w2IVbip2LdBjQs=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1saW50LWFnLTRiN2FiMi1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9LCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzL3RjZy12YXVsdC9BUVZDeWp0WFlTb0tjS3pFaDVQNjNxZXNmMkNOIiwicHJldmlld1VybCI6InRjZy12YXVsdC1naXQtY29udm95LWxpbnQtYWctNGI3YWIyLXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIn1dLCJyZXF1ZXN0UmV2aWV3VXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3ZlcmNlbC1hZ2VudC9yZXF1ZXN0LXJldmlldz9vd25lcj12YXJ1dGFzdSZyZXBvPXRjZy12YXVsdCZwcj0yOSJ9 The latest updates on your projects. Learn more about [Vercel for GitHub](https://vercel.link/github-learn-more). | Project | Deployment | Actions | Updated (UTC) | | :--- | :----- | :------ | :------ | | [tcg-vault](https://vercel.com/randall-stillwells-projects/tcg-vault) | ![Ready](https://vercel.com/static/status/ready.svg) [Ready](https://vercel.com/randall-stillwells-projects/tcg-vault/AQVCyjtXYSoKcKzEh5P63qesf2CN) | [Preview](https://tcg-vault-git-convoy-lint-ag-4b7ab2-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-convoy-lint-ag-4b7ab2-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 27, 2026 3:50am | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=29" rel="noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://agents-vade-review.vercel.sh/request-review-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://agents-vade-review.vercel.sh/request-review-light.svg"><img src="https://agents-vade-review.vercel.sh/request-review-light.svg" alt="Request Review"></picture></a>
github-actions[bot] commented 2026-05-26 23:50:55 -04:00 (Migrated from github.com)

Pipeline Health

Build + CI gates

Gate Status
Vercel build (Preview) pass
CI: Lint pass
CI: Schema map fresh skipped
Preview smoke pass
Visual diff ⏭ skipped or pending

Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build).

Role reports

Role Status
Reviewer report pending
A11y audit pending
Design system audit pending

See individual comments above for details. This rollup updates automatically.

<!-- pipeline-rollup --> ## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ✅ pass | | Visual diff | ⏭ skipped or pending | _Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build)._ ### Role reports | Role | Status | | --- | --- | | Reviewer report | ⏳ pending | | A11y audit | ⏳ pending | | Design system audit | ⏳ pending | See individual comments above for details. This rollup updates automatically.
Sign in to join this conversation.
No description provided.