Skip to content

Upgrade eslint 5 -> 10, move to flat config, and drop the deprecated request stack - #64

Open
chschan wants to merge 2 commits into
masterfrom
cc-update-dependencies
Open

Upgrade eslint 5 -> 10, move to flat config, and drop the deprecated request stack#64
chschan wants to merge 2 commits into
masterfrom
cc-update-dependencies

Conversation

@chschan

@chschan chschan commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Two commits, both dependency work, no behaviour change to the build or the visual suite. Nothing here is pixel-affecting, so this is independent of the baseline regeneration.


1. eslint 5.16.0 → 10.8.0, and flat config

eslint was stuck on a 2019 release, held there by two dependencies nothing in this repo uses:

  • eslint-config-airbnb@17.1.1 — peer eslint@"^4.19.1 || ^5.3.0"
  • gulp-eslint@5.0.0 — hard dependencies: { eslint: "^5.0.1" }

Both .eslintrc files in the widget family extend "standard", never "airbnb". The only occurrences of airbnb anywhere were the package.json lines themselves, so eslint-config-airbnb plus its eslint-plugin-react / eslint-plugin-jsx-a11y companions were dead weight capping the toolchain.

Breaking change

eslint 10 removed .eslintrc support entirely (deprecated in 9; the ESLINT_USE_FLAT_CONFIG=false escape hatch went with it). Config moves to eslint.config.base.js, exported for widget repos to re-export:

// <widget repo>/eslint.config.js
module.exports = require('rhtmlBuildUtils/eslint.config.base')

A widget repo's .eslintrc and .eslintignore stop working, so gulp lint fails until it adds that file. Safe to land now because the other widget repos are pinned to older tags. Upgrade notes are in the README.

Version is 9.0.0, which also corrects package.json having read 8.0.0 while the 8.0.1 tag pointed at master.

Style is preserved, not changed

eslint-config-standard cannot come along — it caps at eslint 8, and its flat-config successor neostandard caps at 9 — and eslint 10 removed the core formatting rules it configured. Style is reproduced with @stylistic/eslint-plugin's customize() factory plus overrides tuned to the existing tree (comma-dangle: never, space-before-function-paren: always, brace-style: 1tbs, arrow-parens / max-statements-per-line off).

No file is reformatted beyond array-bracket-spacing, which eslint-config-standard also flagged and which --fix handled.

One thing worth a close look

The --fix pass silently deleted two eslint-disable directives. no-extend-native and no-new-func come from eslint-config-standard, not js.configs.recommended. With the rules absent, the deliberate directives in esbuildPolyfillShim.js (a polyfill shim — extending natives is its job) and compileRenderContentPage.jest.test.js (uses new Function to prove a round trip) read as unused, and --fix removed them — which would have dropped both rules everywhere, not just at those two sites.

Caught by reviewing the --fix diff. Both rules are now restored explicitly and I verified the directives survive a --fix pass. Flagging the failure mode because it will apply to the widget repos too.

Deliberately out of scope

Kept narrow so this stays an upgrade, not a refactor. Each is a candidate follow-up:

  • no-unused-vars keeps standard's args: 'none' — much of src/tasks takes a gulp parameter it never uses. Removing those dead parameters belongs with the gulp removal.
  • eslint-plugin-promise is registered but its recommended set is not applied. standard only ever enabled promise/param-names; the recommended set flags 14 promise chains.
  • The stylistic/suggestion rules standard had that js.configs.recommended lacks (dot-notation, no-var, prefer-const, prefer-regex-literals, array-callback-return) are off — ~50 pre-existing sites.

Other changes, all forced by the upgrade

  • lint.js shells out to the eslint CLI instead of piping gulp.src through gulp-eslint, matching jestSpecTests.js and bin/prepush.js. This makes the eslint major a package.json concern rather than a code concern, and eslint --fix writes files itself so the gulp-if / isFixed / gulp.dest('.') dance is gone. Only gulp-eslint and gulp-if are dropped — the rest of gulp is untouched, so this does not pre-empt the gulp removal.
  • getBinPath extracted from getJestPath so the lint task shares the windows launcher and widget-repo-first lookup. getJestPath's four existing tests pass unchanged; getBinPath has its own.
  • fancy-log declared. compileES6.js requires it directly, but it was resolving only transitively through gulp — a latent break for anyone trimming gulp. eslint-plugin-n's no-extraneous-require caught this automatically.
  • new Buffer()Buffer.from() at the two sites whose node/no-deprecated-api directives had gone stale (renamed n/no-deprecated-api under eslint-plugin-n).
  • hasOwnPropertyObject.hasOwn in the snapshot runner template.
  • The experiment UI declares sourceType: module for itself. It only parsed before because eslint-config-standard set module for the whole project, including the CommonJS majority. Now-redundant /* global fetch */ comments removed, and n/no-unsupported-features/node-builtins is off there — judging browser code against engines.node reported fetch as unsupported.
  • engines.node raised to ^20.19.0 || ^22.13.0 || >=24, eslint 10's requirement. CI already runs node 22.

2. request-promise → node's global fetch

request and request-promise were both deprecated, and between them owned the only two CRITICAL advisories in npm audit: SSRF in request, plus unsafe random boundary and CRLF injection in the form-data it pulls in. form-data@2.3.3 had exactly one parent (request), and request had only two (this package and request-promise), so dropping both root deps removes the whole subtree.

request-promise was used in exactly one place — fetching the expected state json in checkState. Global fetch covers it with no new dependency.

The explicit response.ok check is not incidental. request-promise defaulted to simple: true and so rejected on a non-2xx response; fetch resolves regardless and only reports status on the response. Without the check, a missing state file would reach .json() as a 404 html body and fail with a JSON parse error naming neither the url nor the status. Verified against a live server:

case result
200 JSON parses correctly
404 rejects naming url and status
connection refused rejects

The fetch call carries a scoped n/no-unsupported-features/node-builtins directive. That rule is about a stability label, not availability: global fetch has been present and enabled by default since node 18, and node only dropped the "experimental" tag in 21. engines.node admits ^20.19.0 because that is eslint 10's floor. Narrowing engines to >=22 purely to silence the rule would drop node 20 for every widget developer — a worse trade than one directive.


Verification

check result
eslint . clean, exit 0
npm test 46/46 pass, 6 suites
node ./bin/prepush exit 0

Against master:

metric before after
vulnerabilities 33 20
critical 2 0
high 12 7
deprecation warnings 16 10

Everything still outstanding is owned by gulp (7 deprecations, and all 7 remaining highs: braces, micromatch, chokidar, glob-watcher, accord, gulp-less, lodash.pick), jest's own tree (glob, inflight), or deep-diff.

🤖 Generated with Claude Code

chschan and others added 2 commits August 5, 2026 16:16
eslint 5.16.0 was held in place by two things that nothing in this repo
actually used: eslint-config-airbnb (peer eslint ^4.19.1 || ^5.3.0) and
gulp-eslint@5 (hard dependency on eslint ^5.0.1). Both .eslintrc files in
the widget family extend "standard", never "airbnb", so the airbnb config
and its eslint-plugin-react / eslint-plugin-jsx-a11y companions were dead
weight capping the toolchain. Removing them lifts the cap; replacing
gulp-eslint lifts the rest.

eslint 10 removed .eslintrc support entirely, so the config moves to flat
config in eslint.config.base.js. That file is exported for widget repos to
re-export from a one line eslint.config.js, which is the breaking part of
this change: a widget repo's .eslintrc and .eslintignore stop working.

eslint-config-standard cannot come along (it caps at eslint 8, and its flat
config successor neostandard caps at 9), and eslint 10 removed the core
formatting rules it configured. The style is reproduced with @Stylistic's
customize() factory plus overrides tuned to the existing tree, so no file
is reformatted beyond array-bracket-spacing, which eslint-config-standard
also flagged and which --fix handled.

Deliberately scoped so this stays a dependency upgrade:
- no-unused-vars keeps standard's args: 'none', because much of src/tasks
  takes a `gulp` parameter it never uses. Removing those dead parameters
  belongs with the gulp removal.
- eslint-plugin-promise is registered but its recommended set is not
  applied. standard only enabled promise/param-names; the recommended set
  flags 14 promise chains that are worth revisiting separately.
- The stylistic/suggestion rules standard had that js.configs.recommended
  lacks (dot-notation, no-var, prefer-const, prefer-regex-literals,
  array-callback-return) are not enabled. Turning them on flags ~50
  pre-existing sites and is a refactor, not an upgrade.

no-extend-native and no-new-func ARE restored explicitly: the tree carries
deliberate eslint-disable directives for both, and with the rules absent
those directives read as unused and `eslint --fix` deletes them, dropping
the rules everywhere else rather than just at those two sites.

Also in this change, all forced by the upgrade:
- lint.js shells out to the eslint CLI instead of piping gulp.src through
  gulp-eslint, matching jestSpecTests.js and bin/prepush.js. eslint --fix
  writes files itself, so the gulp-if/isFixed/gulp.dest('.') dance goes.
  Only gulp-eslint and gulp-if are dropped; the rest of gulp is untouched.
- getBinPath extracted from getJestPath so the lint task shares the
  platform launcher and widget-repo-first lookup. getJestPath's existing
  tests pass unchanged.
- fancy-log declared. compileES6.js requires it directly but it was only
  resolving transitively through gulp; eslint-plugin-n's
  no-extraneous-require caught it.
- new Buffer() -> Buffer.from() at the two sites whose stale
  `node/no-deprecated-api` directives no longer matched the renamed
  n/no-deprecated-api.
- hasOwnProperty -> Object.hasOwn in the snapshot runner template.
- The experiment UI declares sourceType: module for itself. It only parsed
  before because eslint-config-standard set module for the whole project,
  including the CommonJS majority.
- engines.node raised to ^20.19.0 || ^22.13.0 || >=24, which eslint 10
  requires. CI already runs node 22.
- version 9.0.0, which also corrects package.json having read 8.0.0 while
  the 8.0.1 tag pointed at master.

npm audit: 33 vulnerabilities -> 26, high 12 -> 7. The 2 criticals are
request/form-data and are untouched here.

Verified: eslint . clean, 46/46 jest tests pass, bin/prepush exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
request and request-promise were both deprecated, and between them they
owned the only two CRITICAL advisories in npm audit: SSRF in request, plus
unsafe random boundary and CRLF injection in the form-data it pulls in.
form-data@2.3.3 had exactly one parent (request), and request had only two
(this package and request-promise), so dropping both root deps removes the
whole subtree.

request-promise was used in exactly one place: fetching the expected state
json in checkState. Global fetch covers it with no new dependency.

NB the explicit response.ok check is not incidental. request-promise
defaulted to simple: true and so REJECTED on a non-2xx response, whereas
fetch resolves regardless and only reports the status on the response.
Without the check a missing state file would reach .json() as a 404 html
body and fail with a JSON parse error naming neither the url nor the
status. Verified against a live server: 200 parses, 404 rejects naming url
and status, connection refused rejects.

The fetch call carries a scoped n/no-unsupported-features/node-builtins
directive. That rule is about a stability LABEL, not availability: global
fetch has been present and enabled by default since node 18, and node only
dropped the "experimental" tag in 21. engines.node admits ^20.19.0 because
that is eslint 10's floor. Narrowing engines to >=22 purely to silence the
rule would drop node 20 for every widget developer, which is a worse trade
than one directive.

npm audit: 26 vulnerabilities -> 20, and critical 2 -> 0.
Deprecation warnings on a clean install: 12 -> 10.

Cumulative for this branch, against master: 33 vulnerabilities -> 20,
critical 2 -> 0, high 12 -> 7, deprecations 16 -> 10. Everything still
outstanding is owned by gulp (7 deprecations, all 7 highs), jest's own
tree (glob, inflight), or deep-diff.

Verified: eslint . clean, 46/46 jest tests pass, bin/prepush exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chschan chschan changed the title Upgrade eslint 5 -> 10 and move to flat config Upgrade eslint 5 -> 10, move to flat config, and drop the deprecated request stack Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant