CONSOLE-5439: Migrate to eslint 9 - #16878
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@logonoff: This pull request references CONSOLE-5439 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
💤 Files with no reviewable changes (9)
🚧 Files skipped from review as they are similar to previous changes (9)
WalkthroughFrontend linting migrates to ESLint 9 flat configuration. Legacy rule presets and directives are updated or removed, scoped overrides are centralized in ChangesESLint 9 migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ESLintCLI
participant FlatConfig
participant SharedPresets
participant FrontendSources
ESLintCLI->>FlatConfig: Load eslint.config.ts
FlatConfig->>SharedPresets: Resolve updated presets
FlatConfig->>FrontendSources: Apply scoped rules and ignores
FrontendSources-->>ESLintCLI: Produce lint diagnostics
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/label px-approved |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
frontend/eslint.config.ts (1)
107-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTODO carries forward pre-existing rule debt for
public/.This large block intentionally disables rules for
public/**that are enforced elsewhere, matching prior behavior per the TODO. Not a regression from this migration, just flagging the open TODO — happy to help scope enabling these incrementally if useful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/eslint.config.ts` around lines 107 - 165, Preserve the existing public/** rule overrides and update the TODO associated with the public-files ESLint configuration to explicitly track this pre-existing rule debt and future incremental enablement. Do not remove or change the disabled rules as part of this migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/eslint.config.ts`:
- Around line 178-188: Replace the invalid no-namespace entry in the
CYPRESS_FILES rules with `@typescript-eslint/no-namespace` so the intended
TypeScript namespace rule is disabled without triggering an unknown-rule error.
Leave the surrounding rule configuration unchanged.
In `@frontend/packages/eslint-plugin-console/index.js`:
- Around line 48-63: Remove the legacy overrides entry from the preset
configuration in the exported config object of the console ESLint plugin, and
move its TypeScript-specific `@typescript-eslint/no-unused-vars` rule into
frontend/eslint.config.ts as a separate flat-config object using files:
['**/*.ts', '**/*.tsx']. Preserve all existing rule options and confirm the
preset is not relying on FlatCompat.
- Around line 52-59: Update the `@typescript-eslint/no-unused-vars` configuration
to anchor varsIgnorePattern so it matches only the exact React binding, while
preserving the existing unused-variable options and behavior for all other
variable names.
In `@frontend/packages/eslint-plugin-console/lib/config/rules/react-hooks.js`:
- Line 7: Update the react-hooks/exhaustive-deps setting in the ESLint
configuration to remain blocking at error level, or narrowly scope any
warning-level exception to documented legacy paths and enforce it through CI’s
--max-warnings policy.
In `@frontend/packages/eslint-plugin-console/package.json`:
- Line 14: Update the eslint dependency declaration in package.json from ^9.0.0
to ^9.22.0 or higher, ensuring the package contract supports the defineConfig,
globalIgnores, and extends APIs used by frontend/eslint.config.ts. Verify the
resolved eslint version in the yarn lockfile is at least 9.22.0 and update the
lockfile if necessary.
---
Nitpick comments:
In `@frontend/eslint.config.ts`:
- Around line 107-165: Preserve the existing public/** rule overrides and update
the TODO associated with the public-files ESLint configuration to explicitly
track this pre-existing rule debt and future incremental enablement. Do not
remove or change the disabled rules as part of this migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d1224c46-dd18-4f80-babd-26b1d3250e2d
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (42)
frontend/.eslintignorefrontend/.eslintrc.jsfrontend/__mocks__/fetch.tsfrontend/__mocks__/i18next.tsfrontend/__mocks__/react-i18next.tsfrontend/__mocks__/styleMock.jsfrontend/e2e/.eslintrc.cjsfrontend/eslint.config.tsfrontend/get-local-plugins.jsfrontend/i18n-scripts/.eslintrc.jsfrontend/package.jsonfrontend/packages/.eslintrc.jsfrontend/packages/console-dynamic-plugin-sdk/scripts/.eslintrc.jsfrontend/packages/console-dynamic-plugin-sdk/src/webpack/.eslintrc.jsfrontend/packages/console-plugin-sdk/src/codegen/.eslintrc.jsfrontend/packages/console-plugin-sdk/src/webpack/.eslintrc.jsfrontend/packages/console-shared/src/utils/icon-utils.tsfrontend/packages/dev-console/integration-tests/.eslintrcfrontend/packages/dev-console/src/actions/add-resources.tsxfrontend/packages/dev-console/src/components/import/ImportStrategySelector.tsxfrontend/packages/dev-console/src/components/import/__tests__/deployImage-submit-utils.spec.tsfrontend/packages/dev-console/src/components/import/__tests__/render-utils.spec.tsfrontend/packages/dev-console/src/components/import/git/GitTypeSelector.tsxfrontend/packages/dev-console/src/components/import/render-utils.tsxfrontend/packages/dev-console/src/utils/icons.tsxfrontend/packages/eslint-plugin-console/.eslintrcfrontend/packages/eslint-plugin-console/index.jsfrontend/packages/eslint-plugin-console/lib/config/playwright.jsfrontend/packages/eslint-plugin-console/lib/config/rules/airbnb-base-overrides.jsfrontend/packages/eslint-plugin-console/lib/config/rules/react-hooks.jsfrontend/packages/eslint-plugin-console/lib/config/rules/react.jsfrontend/packages/eslint-plugin-console/package.jsonfrontend/packages/helm-plugin/integration-tests/.eslintrcfrontend/packages/integration-tests/.eslintrcfrontend/packages/knative-plugin/integration-tests/.eslintrcfrontend/packages/knative-plugin/src/utils/icons.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/CloudShellMastheadButton.tsxfrontend/public/components/namespace.jsxfrontend/public/components/row-filter.jsxfrontend/public/module/k8s/label-selector.jsfrontend/test-puppeteer-csp.tsfrontend/webpack.circular-deps.ts
💤 Files with no reviewable changes (23)
- frontend/webpack.circular-deps.ts
- frontend/packages/integration-tests/.eslintrc
- frontend/mocks/react-i18next.ts
- frontend/packages/console-plugin-sdk/src/webpack/.eslintrc.js
- frontend/packages/dev-console/integration-tests/.eslintrc
- frontend/i18n-scripts/.eslintrc.js
- frontend/.eslintignore
- frontend/packages/knative-plugin/integration-tests/.eslintrc
- frontend/packages/console-dynamic-plugin-sdk/src/webpack/.eslintrc.js
- frontend/get-local-plugins.js
- frontend/public/components/namespace.jsx
- frontend/public/components/row-filter.jsx
- frontend/packages/.eslintrc.js
- frontend/.eslintrc.js
- frontend/packages/eslint-plugin-console/.eslintrc
- frontend/e2e/.eslintrc.cjs
- frontend/mocks/i18next.ts
- frontend/packages/helm-plugin/integration-tests/.eslintrc
- frontend/mocks/fetch.ts
- frontend/packages/console-plugin-sdk/src/codegen/.eslintrc.js
- frontend/mocks/styleMock.js
- frontend/test-puppeteer-csp.ts
- frontend/packages/console-dynamic-plugin-sdk/scripts/.eslintrc.js
b31a9ee to
3155a00
Compare
Replace legacy .eslintrc/.eslintignore files across frontend, e2e, i18n scripts, SDK, and integration test folders with a single `frontend/eslint.config.js` flat config. The new config preserves existing scoped rule sets (root app, packages, Cypress tests, Playwright e2e, SDK node dirs, eslint plugin package), centralizes ignore patterns, and adds compatibility helpers for FlatCompat plugin/rule handling. Update the frontend eslint script to drop `--ext` so file matching is driven by flat config.
3155a00 to
344d824
Compare
|
/lgtm |
|
Scheduling tests matching the |
|
performed basic testing with claude and verified that a bunch of the eslint rules are still in force |
…t config Replace custom scopeTo() and stripOrphanedPluginRules() helpers with defineConfig's built-in extends key (available since ESLint 9.22.0). defineConfig handles directory scoping via files/ignores, function matcher intersection, and ignores propagation natively. stripOrphanedPluginRules was unnecessary because ESLint 9 silently ignores "off" rules from unregistered plugins. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eact-typescript-prettier preset
Move root-frontend-specific rules (no-restricted-imports, no-restricted-syntax,
react-hooks/exhaustive-deps, etc.) into the react-typescript-prettier preset in
eslint-plugin-console so public/ and packages/ share a single config entry.
A public/-only override disables ~45 airbnb/RTP rules not previously enforced
there. These can be removed incrementally to tighten public/ linting over time.
Other changes:
- Add eslint-disable comments for pre-existing PF Font Awesome icon imports
now caught by the no-restricted-imports rule in packages/
- Remove stale eslint-disable tsdoc/syntax from .js/.jsx files where the
tsdoc plugin is no longer loaded
- Scope tsdoc/syntax to public/**/*.{ts,tsx} only
- Add scripts/ to globalIgnores
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tuent configs Distribute rules from the react-typescript-prettier preset into the configs they logically belong to, so they're inherited through the extends chain rather than duplicated at the preset level. - airbnb-base-overrides: curly, consistent-return, consistent-this, max-nested-callbacks, no-alert, no-constant-condition, no-underscore-dangle, object-shorthand, prefer-const, no-restricted-imports, no-restricted-syntax - react: react/no-string-refs, react/self-closing-comp, react/no-unescaped-entities - react-hooks: exhaustive-deps downgraded from error to warn Also restores tsdoc/syntax to eslint-disable comments in two .tsx files where removing it exposed new warnings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lbacks Consolidate the two Cypress integration test config objects into one using shared files/rules/settings. Disable max-nested-callbacks for Cypress tests (deeply nested cy.then() chains are idiomatic) and suppress the rule in a unit test with pre-existing deep nesting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… config blocks Move Playwright-specific rule overrides (no-console, no-empty-pattern, no-restricted-syntax waitFor warning, playwright/* rules) into the plugin:console/playwright config. Merge the two e2e config blocks in eslint.config.ts into one. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
344d824 to
e33ac7d
Compare
|
/verified by @logonoff |
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@logonoff: This pull request references CONSOLE-5439 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test analyze backend frontend images okd-scos-images |
f9644ce
into
openshift:main
Analysis / Root cause:
ESlint 8 is EOL, ESlint 9 is the oldest version in support (maintenance mode)
Solution description:
.eslintrc/.eslintrc.jsfiles into a singular, centralized flateslint.config.tsTest cases:
Summary by CodeRabbit