Conversation
## Changes
- `package.json`: prepend `DISABLE_V8_COMPILE_CACHE=1` to the `lint` and
`lint-non-build` scripts.
ESLint 8.x bundles `v8-compile-cache`, which globally intercepts module
loading and treats files as CJS. On Node 22 the transitive dep
`async-function@1.0.0` (pulled in via `get-intrinsic` and
`is-async-function`) advertises `module-sync: ./require.mjs` in its
exports, and Node 22 honors that — but `v8-compile-cache` short-circuits
with a CJS compile and crashes on the file's `import` statement, taking
both lint scripts down with a `SyntaxError: Cannot use import statement
outside a module`.
Setting `DISABLE_V8_COMPILE_CACHE=1` is the documented escape hatch and
is a no-op on Node 18 (which CI uses, hence CI was unaffected). The
`.nvmrc` says `lts/*`, which now resolves to Node 22, so any contributor
using current LTS hits this. Both `yarn lint` and `yarn lint-non-build`
return exit 0 after the change (the 248 preexisting `i18next/no-literal-string`
warnings are unrelated).
Bash env-prefix syntax matches the project's existing convention; no
`cross-env` is wired up and no other script uses Windows-shell-aware env
handling. The proper long-term fix is upgrading to ESLint 9 (which
dropped `v8-compile-cache` entirely), tracked separately.
## Process
Sage discovered the failure during a `/review-pr` run on PR 5830, where
`yarn eslint` against a single file crashed before producing output. After
closing that PR, Sage asked to fix ESLint. Claude reproduced the failure,
identified the `v8-compile-cache` × Node 22 module-sync interaction by
reading the failing stack and the `async-function` package.json `exports`
block, traced `async-function` back to `get-intrinsic` and
`is-async-function` via `yarn.lock`, confirmed CI runs on Node 18 (so
this is a local-only regression for current LTS users), and applied the
documented `DISABLE_V8_COMPILE_CACHE=1` workaround. Both scripts pass
cleanly on the first run after the edit.
Session character: short and focused. Sage sent two short messages on
this fix ("now let's fix eslint", "commit the quick fix, then let's work
on the upgrade"). No iteration or false starts; root cause was clear from
the first error message and the fix verified on the first attempt. Two
test runs (one per lint script) after the edit, both green.
(Commit message written by Claude Code.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Changes
- `eslint.config.mjs` (new) replaces `.eslintrc` (deleted). Flat config
required by ESLint 9. Uses `@eslint/js` recommended +
`eslint-plugin-react/configs/flat/recommended` +
`eslint-plugin-jsx-a11y/flatConfigs/recommended` +
`eslint-plugin-import/flatConfigs/recommended` +
`eslint-plugin-i18next/configs/flat/recommended` instead of
`extends: airbnb`. The 60+ rule overrides in `.eslintrc` were almost
entirely suppressing airbnb opinions; with airbnb dropped, those
suppressions are no longer needed. The 15 overrides retained are
either custom enforcement (`radix`, `new-cap`, `no-else-return`,
`no-trailing-spaces`, `no-unused-vars`, `react/jsx-equals-spacing`,
`i18next/no-literal-string`) or rules that come from the
plugin/recommended sets and the team explicitly disables.
Two non-obvious config carries:
- `no-unused-vars` is set with `caughtErrors: 'none'`. ESLint 9
flipped the default to flag unused catch parameters, but the
codebase has 16 catch-swallow sites; preserving the previous
default keeps this migration scoped to "make ESLint 9 work" rather
than "fix latent error handling."
- `import/namespace` is off. The codebase's
`babel-plugin-add-module-exports` interop pattern (test files
using `import * as api` then calling `api.staticMethod()`) can't
be statically verified — 7 test files rely on it.
Resolver uses `eslint-import-resolver-alias` for `~`,
`@components`, `@constants`, `@actions/`, plus
`node.moduleDirectory: ['node_modules', 'app/assets/javascripts']`
to mirror Jest's `moduleDirectories` so test files using bare paths
like `actions/alert_actions` resolve.
- `package.json`: bump `eslint` 8.10 → 9.39.4; bump
`eslint-plugin-import` 2.22 → 2.32; bump `eslint-plugin-jsx-a11y`
6.2 → 6.10; bump `eslint-plugin-i18next` 6.0 → 6.1; bump
`eslint-webpack-plugin` 3.1 → 6.0 (required for ESLint 9). Add
`@eslint/js`, `globals`, `eslint-import-resolver-alias`. Remove
`eslint-config-airbnb`, `@babel/eslint-parser` (espree handles all
syntax in the codebase), `eslint-import-resolver-babel-plugin-root-import`
(unmaintained). Drop the `DISABLE_V8_COMPILE_CACHE=1` workaround
from the `lint` and `lint-non-build` scripts — ESLint 9 dropped
the `v8-compile-cache` dep that necessitated it.
- `webpack.config.js`: remove `threads` and `cache` options from the
`ESLintPlugin` constructor; both were removed in eslint-webpack-plugin
v5. Threading is now auto-managed.
- `.github/workflows/ci.yml`: bump `node-version` from `'18'` to `'22'`.
CI was pinned to 18 only because Node 22 + ESLint 8 + the
`async-function` package's `module-sync` exports tripped the
`v8-compile-cache` shim. With ESLint 9 the trap is gone, and `lts/*`
in `.nvmrc` resolves to 22, so CI now matches what contributors run
locally.
- 29 source files: `eslint --fix` removed inline
`// eslint-disable-line ...` directives that were suppressing
airbnb-only rules (`no-restricted-syntax`, `no-console`,
`no-await-in-loop`, `react/no-did-update-set-state`, `no-new`).
These directives became no-ops once airbnb was dropped; ESLint 9
flagged them as "unused eslint-disable directive." 38 lines edited,
one comment per line removed.
`yarn lint` reports 248 warnings / 0 errors — identical to the
pre-upgrade baseline (all 248 are `i18next/no-literal-string`).
`yarn lint-non-build` is clean. `yarn build` succeeds with the
ESLint webpack plugin running inline. `yarn test` passes (59 suites,
389 tests).
## Process
Sage asked to upgrade after the previous commit landed a
`DISABLE_V8_COMPILE_CACHE=1` quick fix for ESLint 8 + Node 22, with
guidance that the repo isn't very opinionated and that adopting
maintainer defaults of up-to-date components was fine.
The path was: audit ESLint 9 compatibility of every plugin via
`npm view`; decide between adopting `eslint-config-airbnb-extended`
(flat-config airbnb fork) versus dropping airbnb entirely; chose
the latter after observing that the existing `.eslintrc` was
disabling almost every airbnb opinion — plugin/recommended sets are
closer to what's actually enforced. Translated 60+ rule overrides
into a 15-rule keeper list. Wrote the flat config; resolved a
sequence of failures that surfaced over several lint runs:
1. First run: 524 errors. 443 `no-unused-vars` (mostly `'React' is
defined but never used` from including the `jsx-runtime` config
even though Babel still uses classic runtime in this repo) +
73 `no-undef` (jest globals in co-located `*.spec.js` files
under `app/assets/javascripts`, `require` used in webpack
contexts).
2. Removed the `jsx-runtime` config; broadened the test-files glob
to `**/*.spec.{js,jsx}`; added `require` and the team's custom
window globals; added Node globals for spec files.
3. Second run: 29 errors. 21 `no-unused-vars` on catch parameters
(ESLint 9 default change) + 8 `react/display-name` (anonymous
arrow defaults that older airbnb had off).
4. Added `caughtErrors: 'none'` and `react/display-name: 'off'`
suppressions.
5. Third run: 0 errors, 286 warnings (vs 248 baseline). The 38
extras were all "unused eslint-disable directive" cleanup.
`--fix` removed them, bringing the count to exactly 248.
`yarn lint-non-build` had its own surprises: `sinon`/`reduxStore`
globals injected by `test/testHelper.js` weren't in the old jest
env, and 7 `import/namespace` errors exposed the
`babel-plugin-add-module-exports` interop pattern in test files.
The first was added to test-file globals; the second was suppressed
since the runtime works and a real fix would touch ~30 test files.
Webpack threw "Unknown options: threads" on the first `yarn build`
run; eslint-webpack-plugin v5 removed both `threads` and `cache`.
CI Node bump was the cleanup pass — the original v8-compile-cache
trap that pinned CI to Node 18 is gone with ESLint 9.
Session character: extended back-and-forth, ~14 user messages
across three distinct asks (review PR 5830 → close, fix ESLint
quick fix and commit, then ESLint 9 migration). The migration
itself was iterative: ~5 lint runs, each surfacing a different
bucket of issues, each resolved with a targeted config change.
The full JS test suite (`yarn test`) ran once at the end and
passed first try (59 suites, 389 tests). One `yarn build` ran
at the end of the migration, hit the `threads` option error,
fixed and reran clean. No false starts on the strategic
direction; the iteration was all in working through the long tail
of plugin behavior differences.
(Commit message written by Claude Code.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Changes - `package.json`: re-add `@babel/plugin-transform-runtime` to `devDependencies`. It's referenced by `babel.config.js` in the test env (`env.test.plugins`) but isn't a transitive dep of anything, so removing the explicit dev dep made the package unavailable. - `yarn.lock`: regenerated. ## Process CI's "JavaScript test suite" job failed at the `yarn coverage` step with `Cannot find package '@babel/plugin-transform-runtime' imported from babel-virtual-resolve-base.js`. The package was removed in the ESLint 9 migration commit (9b9e5f4) — it was listed adjacent to the ESLint-related deps being deleted, and got swept up in the same edit. The local `yarn test` run during the migration didn't catch the problem because `node_modules` still contained the package from a prior install, masking the missing manifest entry. CI's `yarn install --immutable` pruned strictly per `package.json` and exposed it. `yarn coverage` ran clean locally after restoring the dep. Session character: single-message report from Sage ("this broke on CI"), one round-trip to identify and fix. No iteration needed once the failure log was read. (Commit message written by Claude Code.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.