Skip to content

ESLint migration followups#7774

Open
Steve-Mcl wants to merge 5 commits into
mainfrom
6458-eslint-migration-followups
Open

ESLint migration followups#7774
Steve-Mcl wants to merge 5 commits into
mainfrom
6458-eslint-migration-followups

Conversation

@Steve-Mcl

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

Copy link
Copy Markdown
Contributor

Description

Part 2 of upgrade to eslint migration

The initial PR performed the migration and handled actual errors. This PR handles the 100+ warnings that were temporarily ignored via deliberate/temporary TODOs in the lint config.

Specifics

forge code
Unused eslint-disable directive (no problems were reported from 'no-useless-return').eslint
Unused eslint-disable directive (no problems were reported from 'n/no-unpublished-require').eslint
Unused eslint-disable directive (no problems were reported from 'promise/no-callback-in-promise').eslint

vue files
Unused eslint-disable directive (no problems were reported from 'no-template-curly-in-string').eslint
Unused eslint-disable directive (no problems were reported from 'vue/one-component-per-file').eslint
Unused eslint-disable directive (no problems were reported from 'promise/no-nesting').eslint
Unused eslint-disable directive (no problems were reported from 'n/no-extraneous-import').eslint

frontend test files
Unused eslint-disable directive (no problems were reported from 'n/no-process-exit').eslint
Unused eslint-disable directive (no problems were reported from 'cypress/require-data-selectors').eslint

forge test files
Unused eslint-disable directive (no problems were reported from 'no-unused-vars').eslint
Unused eslint-disable directive (no problems were reported from 'camelcase').eslint

vue props
Prop XXX should be optional.eslintvue/no-required-prop-with-default
Where sensible, either the required was set false (removed) OR the default was removed (on a case by case basis)

These ↑ particular changes (in the vue files to the props) were discussed and agreed with @cstns


CLI warnings:

(node:30692) ESLintEnvWarning: /* eslint-env */ comments are no longer recognized when linting with flat config and will be reported as errors as of v10.0.0. Replace them with /* global */ comments or define globals in your config file. See https://eslint.org/docs/latest/use/configure/migration-guide#eslint-env-configuration-comments for details. Found in packages\flowfuse\test\unit\frontend\stores\context.spec.js at line 1.
(Use `node --trace-warnings ...` to show where the warning was created)
(node:30692) ESLintEnvWarning: /* eslint-env */ comments are no longer recognized when linting with flat config and will be reported as errors as of v10.0.0. Replace them with /* global */ comments or define globals in your config file. See https://eslint.org/docs/latest/use/configure/migration-guide#eslint-env-configuration-comments for details. Found in packages\flowfuse\test\unit\frontend\subscribers\team-channel.subscriber.spec.js at line 1.

Removed the /* eslint-env browser */ directives from both files as they are already covered by eslint.config.mjs:236-244

        files: ['test/unit/frontend/**', 'test/e2e/frontend/**'],
        languageOptions: {
            sourceType: 'module',
            globals: {
                ...globals.browser
            }
        }

Note

Windows cannot run full lint since the addition of flowfuse\scripts\lint-colors.js this will be addressed in a follow up issue / PR

Related Issue(s)

#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

Steve-Mcl added 4 commits July 8, 2026 10:57
**forge code**
Unused eslint-disable directive (no problems were reported from 'no-useless-return').eslint
Unused eslint-disable directive (no problems were reported from 'n/no-unpublished-require').eslint
Unused eslint-disable directive (no problems were reported from 'promise/no-callback-in-promise').eslint

**vue files**
Unused eslint-disable directive (no problems were reported from 'no-template-curly-in-string').eslint
Unused eslint-disable directive (no problems were reported from 'vue/one-component-per-file').eslint
Unused eslint-disable directive (no problems were reported from 'promise/no-nesting').eslint
Unused eslint-disable directive (no problems were reported from 'n/no-extraneous-import').eslint

**frontend test files**
Unused eslint-disable directive (no problems were reported from 'n/no-process-exit').eslint
Unused eslint-disable directive (no problems were reported from 'cypress/require-data-selectors').eslint

**forge test files**
Unused eslint-disable directive (no problems were reported from 'no-unused-vars').eslint
Unused eslint-disable directive (no problems were reported from 'camelcase').eslint
@Steve-Mcl

Copy link
Copy Markdown
Contributor Author

Added a [x] ✅ against DB Migrations since the lint fixes touched some of those files. NOTE they will not be re-executed

Also, odd test fail - 99.99% sure it is unrelated / transient! running tests agian. Captured below for reference:

  24 pending
  1 failing

  1) HA Instance API
       removes ha option:
     AssertionError: expected 'starting' to be undefined
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at Context.<anonymous> (test/unit/forge/ee/routes/ha/index_spec.js:152:88)

@Steve-Mcl Steve-Mcl requested a review from cstns July 8, 2026 11:54
@Steve-Mcl

Copy link
Copy Markdown
Contributor Author

@ppawlowski

how do we get past the Check DB migrations error "Error: Migrations added out of order"

No migrations were added here - only clean up of lint warnings.

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.37%. Comparing base (3a61885) to head (0f93e39).

Files with missing lines Patch % Lines
forge/ee/routes/httpTokens/index.js 0.00% 4 Missing ⚠️
forge/ee/routes/teamBroker/3rdPartyBroker.js 0.00% 2 Missing ⚠️
forge/ee/routes/teamBroker/schema.js 0.00% 2 Missing ⚠️
forge/ee/routes/customHostnames/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/deviceEditor/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/gitops/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/protectedInstance/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/staticAssets/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/tables/index.js 0.00% 1 Missing ⚠️
forge/ee/routes/teamBroker/index.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7774      +/-   ##
==========================================
- Coverage   75.38%   75.37%   -0.02%     
==========================================
  Files         428      428              
  Lines       22539    22539              
  Branches     5944     5944              
==========================================
- Hits        16992    16989       -3     
- Misses       5547     5550       +3     
Flag Coverage Δ
backend 75.37% <25.00%> (-0.02%) ⬇️

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.

@ppawlowski

Copy link
Copy Markdown
Contributor

@ppawlowski

how do we get past the Check DB migrations error "Error: Migrations added out of order"

No migrations were added here - only clean up of lint warnings.

5 migration files have been modified, and they are not the newest migration files stored in the migrations directory - that's why the alarm was raised.
If it is not necessary, revert the changes in these files. Otherwise, we will have to refactor the validation script and check the content of each modification instead of just validating git diff.

@ppawlowski

Copy link
Copy Markdown
Contributor

On the other hand, the Check DB migrations workflow will not prevent merging this PR into the main branch.
However, I don't know how the migration script handles a change to an old migration file.

@hardillb

hardillb commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

However, I don't know how the migration script handles a change to an old migration file.
The forge app will ignore any filenames that are already in the database (or with a date in the name older than the last file in the database)

@Steve-Mcl

Copy link
Copy Markdown
Contributor Author

On the other hand, the Check DB migrations workflow will not prevent merging this PR into the main branch. However, I don't know how the migration script handles a change to an old migration file.

To back up what @hardillb states, the forge application uses the date in the file name to determine is a migration is executed. These are stored in the DB once ran. In short, these changes are a no-op

Otherwise, we will have to refactor the validation script and check the content of each modification instead of just validating git diff.

This is a very uncommon change - so lets not fix up the script just now.

On the other hand, the Check DB migrations workflow will not prevent merging this PR into the main branch.

Thanks for the nod.


With all said, I am happy for this to be reviewed & merged as is.

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.

3 participants