Skip to content

chore: Upgrade eslint to v9#7760

Merged
Steve-Mcl merged 12 commits into
mainfrom
6458-eslint-upgrade
Jul 7, 2026
Merged

chore: Upgrade eslint to v9#7760
Steve-Mcl merged 12 commits into
mainfrom
6458-eslint-upgrade

Conversation

@Steve-Mcl

@Steve-Mcl Steve-Mcl commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

As part of getting dependencies up to date (as best as possible / sensible), this is an outstanding task that has been on the backlog for some time. Upgrading to v9 (and flat file format) helps us het past some old dependencies and clear some vulns.

Overview

Migrates the package from ESLint 8 (.eslintrc, eslintrc format) to ESLint 9 flat config (eslint.config.mjs). The Standard ruleset moves from eslint-config-standard to its flat-config successor neostandard, and the Cypress and Vue plugins are brought up to their current majors.

TLDR

I appreciate there is a lot of info below - i have tried to keep it readable / formatted as glanceable info - however it serves as a reference for the what and why!

The long and short is:

  • Updated to eslint v9
    • rewrote config file as mjs file
    • some of the plugins and child deps changed / moved - so the table below lists these out and why!
  • This uncovered errors (and lots of warnings)
    • so this PR focused on getting the upgrade done with minimum set of changes to get it passing (up to keep this PR lighter(er))
    • this meant disabling the new warnings - a follow PR will be made to remove the TODOs in the eslint config and remove the lint errors themselves

Dependency changes (devDependencies)

Upgraded

Package Before After
eslint ^8.48.0 ^9.39.4
eslint-plugin-cypress 2.15.1 ^6.4.2
eslint-plugin-vue ^9.19.2 ^10.9.2
eslint-plugin-no-only-tests ^3.1.0 ^3.4.0

Added

Package Version Why
neostandard ^0.13.0 Flat-config successor to eslint-config-standard; bundles the n, promise, and @stylistic rule sets
@eslint/js ^9.39.4 Flat-config js.configs.recommended base
eslint-plugin-import-x ^4.17.1 Flat-config import rules (import-x/*); replaces eslint-plugin-import
globals ^17.7.0 Named global sets for flat config (node, browser, mocha, …)
vue-eslint-parser ^10.4.1 Now a peer dependency of eslint-plugin-vue v10 (was bundled in v9)

Removed

Package Was Reason
eslint-config-standard ^17.1.0 Replaced by neostandard
eslint-plugin-import ^2.28.1 Replaced by eslint-plugin-import-x
eslint-plugin-n ^16.0.2 Now provided by neostandard
eslint-plugin-promise ^6.1.1 Now provided by neostandard

(@typescript-eslint/parser and @typescript-eslint/eslint-plugin are unchanged and already flat-config compatible.)

Config / script changes

  • .eslintrceslint.config.mjs - The flat config attempts to faithfully port the old rules, overrides, and per-file exceptions.
  • Lint scripts simplified - ESLint 9 auto-discovers eslint.config.mjs, so the explicit config path, globs, and ignore patterns are no longer needed:
    • lint:eslint: eslint -c .eslintrc "forge/**/*.js" "frontend/**/*.js" "frontend/**/*.vue" "test/**/*.js" --ignore-pattern "frontend/dist/**"eslint forge frontend test
    • lint:fix: same simplification, with --fix

Lint errors fixed

Pre-existing code issues surfaced by the stricter ESLint 9 / neostandard rule set, fixed in this PR (all behaviour-preserving):

File Rule Fix
forge/comms/devices.js no-object-constructor The recommendation is to use {} literal instead of new Object() however, for now, I have silenced the rule for this one item.
forge/routes/api/search.js no-constant-binary-expression Dropped dead ?? 0 guards - Array.length and its sum are never nullish
frontend/src/api/teamTypes.js no-unsafe-optional-chaining Removed optional chaining before .split(); the !isFree branch guarantees billing.description exists
frontend/src/pages/admin/Teams.vue no-dupe-else-if Removed/corrected the duplicate else-if branch
frontend/src/pages/instance/Settings/Maintenance.vue no-constant-binary-expression, no-unsafe-optional-chaining Same ?? / optional-chaining cleanup
frontend/src/pages/team/changeType.vue no-constant-binary-expression Dropped dead ?? false - !expr is always boolean, never nullish
frontend/src/services/post-message.service.ts @stylistic/block-spacing, @stylistic/object-curly-spacing Added spaces inside object braces

Deferred to a follow-up PR

To keep this PR focused on the migration, two categories of findings are temporarily suppressed in eslint.config.mjs (both tagged TODO(eslint-v9-follow-up)):

  • Stale eslint-disable directives - the rule-set change left inline eslint-disable comments that no longer match a reported problem (for n/no-process-exit, n/no-extraneous-import, n/no-unpublished-require, n/handle-callback-err, no-unused-vars, no-useless-return, no-template-curly-in-string, promise/no-callback-in-promise, vue/one-component-per-file). Silenced via linterOptions.reportUnusedDisableDirectives: 'off'; the follow-up will delete the stale directives and re-enable the check.
  • vue/no-required-prop-with-default - a new Vue 10 rule flagging a required prop that also declares a default. Rule disabled for now; the follow-up will fix the offending prop(s).
  • Stale /* eslint-env */ comments - flat config no longer honours /* eslint-env */ (the globals are already provided by the config's files-scoped languageOptions.globals), so these emit an ESLintEnvWarning on stderr and will become hard errors in ESLint v10. Two files remain (test/unit/frontend/stores/context.spec.js, test/unit/frontend/subscribers/team-channel.subscriber.spec.js); the follow-up will delete the now-redundant /* eslint-env browser */ lines. No config change needed.

Notes for reviewers

  • Frontend .ts files are now linted (the old glob-based script never included them); the Node-resolution rules (n/no-missing-import, n/file-extension-in-import, etc.) are disabled for frontend blocks since it's webpack-bundled, matching the intent of the original config.
  • The Cypress v6 and Vue v10 recommended sets are stricter than the previous versions, so this PR surfaces additional lint findings in test/e2e/** and .vue files. Those are addressed separately.

Related Issue(s)

closes #6458

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.39%. Comparing base (8af93d7) to head (be20378).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7760      +/-   ##
==========================================
- Coverage   75.39%   75.39%   -0.01%     
==========================================
  Files         428      428              
  Lines       22540    22539       -1     
  Branches     5945     5944       -1     
==========================================
- Hits        16994    16993       -1     
  Misses       5546     5546              
Flag Coverage Δ
backend 75.39% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Steve-Mcl

Copy link
Copy Markdown
Contributor Author

@ppawlowski @hardillb

Re: the Trivy/SAST findings on this PR

TL;DR - these are pre-existing vulnerabilities on main, not introduced by this PR.

This is an ESLint 8→9 migration; the only production/lockfile impact is dev-time lint tooling. None of the flagged packages (vite, serialize-javascript, uuid, jsonata, minimatch) were added or changed here.

Details

  • All five flagged versions already exist in origin/main's own lockfile
    (verified against the tip of main, which is merged into this branch).
  • This PR changes zero lockfile lines for vite / serialize-javascript /
    uuid / jsonata. vite@6.4.2 is byte-identical between main and this branch.
  • They also appear in an npm audit taken before this work started.

Why is it flagging here?
Apparently Trivy scans the entire package-lock.json, not just the PR diff - it reports every vulnerable package present in the file and anchors each to a line number.
Because this PR rewrites large parts of the lockfile (dependency changes + re-ordering), the file counts as "changed", so the scan runs and surfaces the full pre-existing backlog. The SAST workflow also runs on push to main, so main's own SAST run reports the identical set - confirming these aren't new.

Resolution
I'd propose merging this PR on the basis that it introduces none of these findings.

Alternatively, I can add a child PR based off this branch that squashes all/most of these (with
one exception: jsonata it is a transitive child dependency of Node-RED (@node-red/util
→ used only in test tooling). Once clean, merge down to re-run tests and (hopefully) clear the trivy errors?

Or - suggestions welcome :)


FWIW, as stated in sprint planning, I am attempting to get a more healthy npm audit (that includes dev deps) as part of dependabot duties.

ESLINT was an outstanding task that would smash a good handful of warnings and why it was approached first.

@Steve-Mcl
Steve-Mcl requested a review from cstns July 6, 2026 18:53
@ppawlowski

Copy link
Copy Markdown
Contributor

Apparently Trivy scans the entire package-lock.json

This is correct.

these are pre-existing vulnerabilities on main, not introduced by this PR

This is correct. If you check all alerts for this branch, you'll see none were introduced there.

I'd propose merging this PR on the basis that it introduces none of these findings.

That's the best approach.

@cstns cstns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well structured migration with thorough documentation.

the /* eslint-env browser */ lines in context.spec.js and team-channel.subscriber.spec.js were already redundant (the config covers test/unit/frontend/**). Could just delete them here instead of deferring, if you want

@Steve-Mcl
Steve-Mcl enabled auto-merge (squash) July 7, 2026 16:24
@Steve-Mcl

Copy link
Copy Markdown
Contributor Author

well structured migration with thorough documentation.

the /* eslint-env browser */ lines in context.spec.js and team-channel.subscriber.spec.js were already redundant (the config covers test/unit/frontend/**). Could just delete them here instead of deferring, if you want

Wanted to keep the focus on migration (+ error fixes)

Will address in follow up

@Steve-Mcl

Steve-Mcl commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@ppawlowski This has been approved and auto merge is "on" but I will not be around to baby sit (and dont know what happens with regards trivy - will the PR merge or is there a tick box task to do)?

@ppawlowski

Copy link
Copy Markdown
Contributor

Trivy is not a blocker for the auto-merge, but tests are, and they currently fail against changes in this PR.

@Steve-Mcl
Steve-Mcl merged commit 3a61885 into main Jul 7, 2026
35 of 38 checks passed
@Steve-Mcl
Steve-Mcl deleted the 6458-eslint-upgrade branch July 7, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update eslint to v9

3 participants