CONSOLE-5425: Migrate to rspack#16761
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@logonoff: This pull request references CONSOLE-5425 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. |
|
/label px-approved |
66f9d26 to
0d9b690
Compare
|
/pipeline required |
|
Scheduling tests matching the |
|
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:
WalkthroughThe frontend build system migrates from Webpack to Rspack, including scripts, plugins, loaders, module federation, analysis, dependency checking, and circular-dependency reporting. Related imports, runtime asset registration, configuration, tests, and documentation are updated. ChangesRspack frontend migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, rhamilto 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/.swcrc`:
- Around line 16-20: Preserve the existing jsc.target setting in the .swcrc
configuration used by `@swc/jest`, or move the complete SWC configuration,
including jsc.target, into the Jest transform; ensure test transpilation retains
the current target behavior.
In `@frontend/public/components/app.tsx`:
- Around line 467-468: Update the service-worker registration in the app
initialization flow to use the root-level
`${window.SERVER_FLAGS.basePath}load-test.sw.js` URL instead of placing the
worker under `static/`, so its scope can cover the application.
In `@frontend/scripts/knip.ts`:
- Line 217: Update the Knip configuration’s entry list to retain
frontend/public/components/app.tsx alongside load-test.sw.js, ensuring the
primary console app tree is analyzed.
In `@frontend/webpack.circular-deps.ts`:
- Around line 83-86: Reset the shared cycles collection at the start of every
compilation by clearing it in compiler.hooks.thisCompilation before
CircularCheckRspackPlugin.onDetected can append entries. Preserve the existing
cycle recording behavior and exclude configuration.
In `@README.md`:
- Line 204: Update the README sentence describing frontend bundles to use the
hyphenated compound adjective “self-contained,” without changing the surrounding
documentation.
🪄 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: daefffd8-8fe9-4ac8-9b66-4ba79f5cd42a
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
.claude/skills/plugin-api-review/SKILL.md.claude/skills/skill-audit/SKILL.mdAGENTS.mdINTERNATIONALIZATION.mdREADME.mdanalyze.shfrontend/.eslintrc.jsfrontend/.swcrcfrontend/__tests__/sdk-dist-imports.spec.tsfrontend/package.jsonfrontend/public/components/RBAC/bindings.tsxfrontend/public/components/api-explorer.tsxfrontend/public/components/app.tsxfrontend/public/components/group.tsxfrontend/public/components/limit-range.tsxfrontend/public/components/persistent-volume-claim.tsxfrontend/public/components/prometheus.tsxfrontend/public/components/utils/resource-log.tsxfrontend/rspack.config.mtsfrontend/scripts/knip.tsfrontend/tsconfig.jsonfrontend/webpack.circular-deps.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@INTERNATIONALIZATION.md`:
- Line 86: Update the registration guidance in INTERNATIONALIZATION.md to
reference the ns array in public/i18n.js without including a hard-coded line
number, preserving the instruction to add the new namespace there.
🪄 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: c05b6ac4-f130-41d3-9777-1e7f291e8900
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
.claude/skills/plugin-api-review/SKILL.md.claude/skills/skill-audit/SKILL.mdAGENTS.mdINTERNATIONALIZATION.mdREADME.mdanalyze.shfrontend/.eslintrc.jsfrontend/.swcrcfrontend/__tests__/sdk-dist-imports.spec.tsfrontend/package.jsonfrontend/public/components/RBAC/bindings.tsxfrontend/public/components/api-explorer.tsxfrontend/public/components/app.tsxfrontend/public/components/group.tsxfrontend/public/components/limit-range.tsxfrontend/public/components/persistent-volume-claim.tsxfrontend/public/components/prometheus.tsxfrontend/public/components/utils/resource-log.tsxfrontend/rspack.config.mtsfrontend/scripts/knip.tsfrontend/tsconfig.jsonfrontend/webpack.circular-deps.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- frontend/tests/sdk-dist-imports.spec.ts
- frontend/public/components/persistent-volume-claim.tsx
- frontend/public/components/app.tsx
- frontend/.swcrc
- analyze.sh
- AGENTS.md
- frontend/tsconfig.json
- frontend/scripts/knip.ts
- frontend/public/components/utils/resource-log.tsx
- frontend/public/components/limit-range.tsx
- frontend/public/components/group.tsx
- frontend/public/components/prometheus.tsx
- frontend/webpack.circular-deps.ts
- frontend/public/components/api-explorer.tsx
- .claude/skills/plugin-api-review/SKILL.md
- frontend/package.json
- frontend/rspack.config.mts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.claude/skills/skill-audit/SKILL.md:
- Line 51: Update the skill-audit instructions to audit only the explicitly
requested skill and metadata supplied as input. Remove directions to enumerate,
read, or cross-reference other entries under .claude/skills/ or any agent
configuration directories, while preserving the existing `@console/internal`
alias-resolution guidance.
🪄 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: f859ff4a-93cb-42a0-a2c7-35ecef831e61
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
.claude/skills/plugin-api-review/SKILL.md.claude/skills/skill-audit/SKILL.mdAGENTS.mdINTERNATIONALIZATION.mdREADME.mdanalyze.shfrontend/.eslintrc.jsfrontend/.swcrcfrontend/__tests__/sdk-dist-imports.spec.tsfrontend/package.jsonfrontend/public/components/RBAC/bindings.tsxfrontend/public/components/api-explorer.tsxfrontend/public/components/app.tsxfrontend/public/components/group.tsxfrontend/public/components/limit-range.tsxfrontend/public/components/persistent-volume-claim.tsxfrontend/public/components/prometheus.tsxfrontend/public/components/utils/resource-log.tsxfrontend/rspack.config.mtsfrontend/scripts/knip.tsfrontend/tsconfig.jsonfrontend/webpack.circular-deps.ts
💤 Files with no reviewable changes (1)
- frontend/.swcrc
🚧 Files skipped from review as they are similar to previous changes (19)
- frontend/tsconfig.json
- frontend/tests/sdk-dist-imports.spec.ts
- .claude/skills/plugin-api-review/SKILL.md
- analyze.sh
- frontend/public/components/app.tsx
- frontend/public/components/group.tsx
- frontend/.eslintrc.js
- AGENTS.md
- README.md
- frontend/public/components/api-explorer.tsx
- frontend/public/components/prometheus.tsx
- INTERNATIONALIZATION.md
- frontend/scripts/knip.ts
- frontend/public/components/utils/resource-log.tsx
- frontend/public/components/limit-range.tsx
- frontend/public/components/RBAC/bindings.tsx
- frontend/webpack.circular-deps.ts
- frontend/public/components/persistent-volume-claim.tsx
- frontend/rspack.config.mts
3d69f1a to
8e105ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/public/components/group.tsx`:
- Line 7: The imports in frontend/public/components/group.tsx at lines 7-7 and
frontend/public/components/limit-range.tsx at lines 6-11 should avoid the
../module/k8s barrel: update group.tsx to import TableColumn and K8sResourceKind
from their specific declaration modules, and update limit-range.tsx to import
TableColumn from its specific declaration module; leave other imports unchanged.
🪄 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: 3adba50b-ecfa-44c4-a555-b7b5d1f3ab8c
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
.claude/skills/plugin-api-review/SKILL.md.claude/skills/skill-audit/SKILL.mdAGENTS.mdINTERNATIONALIZATION.mdREADME.mdanalyze.shfrontend/.eslintrc.jsfrontend/.swcrcfrontend/__tests__/sdk-dist-imports.spec.tsfrontend/package.jsonfrontend/public/components/RBAC/bindings.tsxfrontend/public/components/api-explorer.tsxfrontend/public/components/app.tsxfrontend/public/components/group.tsxfrontend/public/components/limit-range.tsxfrontend/public/components/persistent-volume-claim.tsxfrontend/public/components/prometheus.tsxfrontend/public/components/utils/resource-log.tsxfrontend/rspack.config.mtsfrontend/scripts/knip.tsfrontend/tsconfig.jsonfrontend/webpack.circular-deps.ts
💤 Files with no reviewable changes (1)
- frontend/.swcrc
🚧 Files skipped from review as they are similar to previous changes (18)
- README.md
- AGENTS.md
- frontend/public/components/utils/resource-log.tsx
- frontend/public/components/app.tsx
- frontend/public/components/persistent-volume-claim.tsx
- analyze.sh
- frontend/.eslintrc.js
- frontend/tests/sdk-dist-imports.spec.ts
- frontend/public/components/prometheus.tsx
- frontend/scripts/knip.ts
- INTERNATIONALIZATION.md
- frontend/tsconfig.json
- .claude/skills/plugin-api-review/SKILL.md
- frontend/public/components/api-explorer.tsx
- frontend/webpack.circular-deps.ts
- frontend/public/components/RBAC/bindings.tsx
- frontend/package.json
- frontend/rspack.config.mts
|
/pipeline required |
|
Scheduling tests matching the |
28dd79f to
73910f1
Compare
fd62625 to
32a6120
Compare
|
/pipeline required |
|
Scheduling tests matching the |
|
/pipeline required |
|
Scheduling tests matching the |
|
/test e2e-playwright |
|
/test e2e-playwright-techpreview |
| "scripts": { | ||
| "postinstall": "yarn ts-node ./scripts/check-patternfly-modules.ts && yarn prepare-husky", | ||
| "clean": "rm -rf ./public/dist", | ||
| "dev": "yarn clean && yarn build-plugin-sdk && REACT_REFRESH=true NODE_OPTIONS=--max-old-space-size=4096 yarn ts-node ./node_modules/.bin/webpack serve --mode=development --progress", |
There was a problem hiding this comment.
Is progress indication the default is rspack?
There was a problem hiding this comment.
Yes it's in the rspack config
| "webpack-bundle-analyzer": "^5.1.1", | ||
| "webpack-cli": "^6.0.1", | ||
| "webpack-dev-server": "5.2.5" | ||
| "webpack": "^5.100.0" |
There was a problem hiding this comment.
Do we need this for anything beyond parsing this dependency when generating Console plugin SDK package manifests?
There was a problem hiding this comment.
It's needed for types and @cypress/webpack-preprocessor
| }), | ||
| new webpack.container.ModuleFederationPlugin({ | ||
| new container.ModuleFederationPluginV1({ | ||
| name: 'bridge', |
There was a problem hiding this comment.
Bridge is the server name, for Console frontend we could use e.g. console
There was a problem hiding this comment.
IIUC name might create a global object window[name], and window['console'] would obviously conflict, so I went with "bridge"
| context: path.resolve(__dirname), | ||
| }, | ||
| { | ||
| from: 'public/locales/**/*.json', |
There was a problem hiding this comment.
This is because we can't use {packages/*,public}/locales expression with CopyRspackPlugin ?
| filter: (p) => path.extname(p) === '.json', | ||
| }, | ||
| { from: 'public/imgs', to: 'imgs' }, | ||
| { from: 'public/load-test.sw.js', to: 'load-test.sw.js' }, |
There was a problem hiding this comment.
OK so this explains my earlier question, I wonder why we didn't do it this way before.
| ], | ||
| devtool: 'cheap-module-source-map', | ||
| stats: 'minimal', | ||
| lazyCompilation: false, // ExtensionValidatorPlugin does not work with this setting enabled |
There was a problem hiding this comment.
We should file an issue to track this and fix ExtensionValidatorPlugin code.
There was a problem hiding this comment.
I'm not sure it's fixable because ExtensionValidatorPlugin needs a complete compilation but lazyCompilation will only compile what is rendered in the browser
|
|
||
| // public/ is the @console/internal workspace (has its own package.json) | ||
| public: { | ||
| entry: ['components/app.tsx', 'load-test.sw.js'], |
There was a problem hiding this comment.
Why is Console app entry point removed here?
There was a problem hiding this comment.
knip can parse rspack config and it reads the entry point from entry.main now. load-test.sw.js is still an entry point just to suppress the "unused file" knip warning
| "e2e" | ||
| ], | ||
| "include": ["**/*.js", "**/*.jsx", "**/*.ts", "**/*.tsx", "**/*.json"] | ||
| "include": ["**/*.js", "**/*.jsx", "**/*.mts", "**/*.ts", "**/*.tsx", "**/*.json"] |
There was a problem hiding this comment.
I think we should consider finding way to represent rspack config as .ts but it can be done as follow-up
3642af3 to
a743704
Compare
|
/pipeline required |
|
Scheduling tests matching the |
|
/remove-label acknowledge-critical-fixes-only |
- Replace webpack with rspack using builtin:swc-loader for JS/TS transpilation and CssExtractRspackPlugin for CSS extraction - Split CopyRspackPlugin locales glob into separate patterns and filter to *.json only (rspack doesn't support brace expansion) - Use html-webpack-plugin for HTML templates (rspack's built-in HTML plugin doesn't handle Go template syntax) - Use css-loader instead of rspack's native CSS module type, which is incompatible with CssExtractRspackPlugin - Parse REACT_REFRESH env var as boolean for builtin:swc-loader - Disable lazy compilation so ExtensionValidator has all modules at emit - Replace file-loader inline syntax with CopyRspackPlugin for the load-test service worker - Configure explicit LightningCSS minimizer with modern browser targets to prevent :is() -> :-webkit-any() expansion that doubled CSS size - Deduplicate PatternFly CSS via resolve.alias since vendor.scss already imports all PF styles - Switch ESLint import resolver to typescript to support exports field - Entrypoint: 7.3 MiB (vs webpack 8.8 MiB) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@logonoff: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Analysis / Root cause:
Rspack is almost twice as fast than webpack for building so we should use it
Solution description:
Test cases:
yarn dev,yarn build,yarn dev-once,yarn check-cycles, andyarn analyzeshould all still workSummary by CodeRabbit
${basePath}load-test.sw.jsstatic path..mtsincludes, SWC config/Jest parsing, knip, analysis wording).