OCPBUGS-66345: Fix broken API exports #15802
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@logonoff: This pull request references Jira Issue OCPBUGS-66345, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
openshift-ci-robot
added
the
jira/valid-bug
logonoff
changed the title
WalkthroughAdds API export presence tests; rebinds and renames a topology export; converts several runtime Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts (2)
🔇 Additional comments (10)
Comment |
openshift-ci
bot
added
component/sdk
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
from
4dedf85 to
0687461
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts (1)
66-67: Breaking rename of topology modify-application API; consider a compat aliasExporting
useModifyApplicationActiontyped asGetModifyApplicationActionis fine, but this is a breaking change for consumers of the formergetModifyApplicationActionexport. If you want to ease migration for existing plugins, consider also exporting a deprecated alias:// keep until next major / deprecation window export const getModifyApplicationAction = useModifyApplicationAction;and call it out in the main SDK changelog / docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/console-dynamic-plugin-sdk/src/api/tests/api.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts
🧬 Code graph analysis (1)
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (1)
GetModifyApplicationAction(287-291)
🔇 Additional comments (2)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md (1)
13-16: Changelog entry and references look consistentThe new prerelease entry and added OCPBUGS/PR references follow the existing format and accurately describe the webpack-related changes in this PR.
Also applies to: 105-105, 123-123
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts (1)
58-59: UtilizationBody and QuickStartsLoader exports correctly bound to named exportsChanging these to use the named module properties rather than
.defaultaligns with how these components are exposed upstream and should fix the broken exports without altering the SDK’s public API shape.Also applies to: 70-71
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
2 times, most recently
from
e378227 to
dc2d733
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/packages/console-dynamic-plugin-sdk/src/api/tests/api.spec.ts
- frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts
- frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md
Outdated
Show resolved
Hide resolved
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
the
acknowledge-critical-fixes-only
openshift-ci-robot
added
the
verified
|
@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 Jira Issue OCPBUGS-66345, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
| export const getModifyApplicationAction: GetModifyApplicationAction = require('@console/topology/src/actions/modify-application') | ||
| .getModifyApplicationAction; | ||
| export const useGetModifyApplicationAction: GetModifyApplicationAction = require('@console/topology/src/actions/modify-application') | ||
| .useGetModifyApplicationAction; |
There was a problem hiding this comment.
@pcbailey @adamviktora Hi, do you know if this breaks Kubevirt plugin's topology integration?
There was a problem hiding this comment.
The actual code change is already merged in Console 4.21 branch, so we'll need to update Kubevirt plugin 4.21 code to use a newer version of @openshift-console/dynamic-plugin-sdk-internal package.
There was a problem hiding this comment.
@vojtechszocs That function is used in src/views/topology/hooks/useModifyApplicationActionProvider.ts, so I would assume this is a breaking change. It should be a simple fix, though.
There was a problem hiding this comment.
@pcbailey Thanks, I'll post a fix for Kubevirt plugin once Console 4.21 plugin SDK packages are published.
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
from
dc2d733 to
c51deac
Compare
openshift-ci-robot
removed
the
verified
|
/verified by api.spec.ts |
openshift-ci-robot
added
the
verified
|
@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. |
openshift-ci-robot
added
the
verified
|
@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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/webpack.circular-deps.ts (1)
108-115: Simplify redundant condition.The condition
if (cycles.length > 0)on line 109 is redundant since the code already returns early whencycles.length === 0at line 93. At this point,cycles.lengthis guaranteed to be greater than zero.Apply this diff to remove the redundant check:
const minLengthCycles = minLengthCycleCount(cycles); - if (cycles.length > 0) { - compilation.errors.push( - new compiler.webpack.WebpackError( - `${HandleCyclesPluginName}: (${cycles.length}) total cycles detected, ${minLengthCycles} of which are min-length :-(`, - ), - ); - } + compilation.errors.push( + new compiler.webpack.WebpackError( + `${HandleCyclesPluginName}: (${cycles.length}) total cycles detected, ${minLengthCycles} of which are min-length :-(`, + ), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts(1 hunks)frontend/webpack.circular-deps.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts
- frontend/packages/console-dynamic-plugin-sdk/src/api/tests/api.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/webpack.circular-deps.ts
🔇 Additional comments (5)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md (1)
13-15: LGTM! Changelog entry is well-formatted and accurate.The changelog correctly documents the removal of direct webpack imports in favor of
compiler.webpackfor better rspack compatibility. The date and references are properly formatted.Also applies to: 105-105, 123-123
frontend/webpack.circular-deps.ts (2)
4-4: LGTM! Type-only import aligns with rspack compatibility goals.Switching to
import typeensures webpack is only used for type references, with runtime usage delegated tocompiler.webpack.
105-105: LGTM! Error signaling and WebpackError usage are correct.The change to red console output appropriately signals an error condition, and using
compiler.webpack.WebpackErrorensures rspack compatibility.Also applies to: 111-111
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts (1)
58-59: LGTM! Named export fixes resolve broken API imports.Switching from
.defaultto named exports (.UtilizationBodyand.QuickStartsLoader) correctly fixes the broken imports as indicated in the PR title.Also applies to: 70-71
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md (1)
13-26: LGTM! Comprehensive changelog with clear breaking change documentation.The changelog entries are well-formatted and clearly document deprecations, type-breaking changes, and enhancements with appropriate markers and references.
Also applies to: 152-152, 220-220
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
from
ba269a3 to
fd5b117
Compare
openshift-ci-robot
removed
the
verified
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
2 times, most recently
from
27a9c0a to
ef04d8a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/webpack.circular-deps.ts (1)
4-4: Unconditional cycle error emission looks correct; inner length check is redundantThe preset now consistently errors when any cycles are detected, and using
compiler.webpack.WebpackErroravoids a hard runtime dependency on thewebpackpackage from this module. Given the earlyif (cycles.length === 0) return;, the laterif (cycles.length > 0)guard is redundant and could be dropped to slightly simplify the emit hook.Also applies to: 105-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (11)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/validation/ExtensionValidator.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/validation/__tests__/ExtensionValidator.spec.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/webpack/ExtensionValidatorPlugin.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/webpack/loaders/dynamic-module-import-loader.ts(1 hunks)frontend/webpack.circular-deps.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/packages/console-dynamic-plugin-sdk/src/webpack/loaders/dynamic-module-import-loader.ts
- frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts
- frontend/packages/console-dynamic-plugin-sdk/src/webpack/ExtensionValidatorPlugin.ts
- frontend/packages/console-dynamic-plugin-sdk/src/validation/ExtensionValidator.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.tsfrontend/webpack.circular-deps.tsfrontend/packages/console-dynamic-plugin-sdk/src/validation/__tests__/ExtensionValidator.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.tsfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md
🧬 Code graph analysis (1)
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts (2)
frontend/packages/topology/src/actions/modify-application.ts (1)
useGetModifyApplicationAction(6-34)frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (1)
GetModifyApplicationAction(287-291)
🔇 Additional comments (6)
frontend/packages/console-dynamic-plugin-sdk/src/validation/__tests__/ExtensionValidator.spec.ts (1)
1-1: Type-only webpack import matches usage patternThe test file only relies on webpack types, so converting the import to
import typekeeps runtime decoupled from webpack and is consistent with the rest of the file.frontend/packages/console-dynamic-plugin-sdk/src/api/__tests__/api.spec.ts (1)
1-18: Per-export definition checks are a good guard against broken APIsIterating
Object.entriesfor both core and internal APIs and asserting each export is defined gives broad coverage for accidental export breakage with minimal maintenance cost; this looks solid.frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md (1)
13-16: Changelog entry clearly documents the webpack import refactorThe new 4.21.0-prerelease.1 section, including the note about replacing direct
webpackimports withcompiler.webpackand the added OCPBUGS/PR references, is consistent with prior entries and makes the behavioral change easy to track.Also applies to: 105-105, 123-123
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-topology-api.ts (1)
66-67: Export rename correctly targets the hook implementationRebinding to
useGetModifyApplicationActionwhile keeping theGetModifyApplicationActiontype maintains the expected signature and fixes the underlying export wiring to the topology action.frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts (1)
13-13: compiler.webpack usage keeps plugin runtime-coupled to the active compilerShifting the webpack import to type-only and routing runtime behavior through
compiler.webpack.WebpackErrorandcompiler.webpack.NormalModule.getCompilationHookscleanly decouples this plugin from a directwebpackmodule dependency while preserving existing behavior.Also applies to: 440-441, 460-460
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md (1)
13-26: Core SDK changelog accurately summarizes the breaking and behavioral changesThe new 4.21.0-prerelease.1 entry clearly calls out deprecations, type-breaking removals, re-exports, and the ResourceYAMLEditor improvements, with appropriate CONSOLE/PR references; this should give plugin authors sufficient guidance for upgrading.
Also applies to: 152-152, 220-220
Instead of directly requiring `webpack`, we can use `compiler.webpack` which ensures that when non-webpack bundlers (i.e., rspack) are used, the correct method is being called. Also, remove support for `CircularDependencyPreset.thresholds` because it's unused and I never want to see this config option enabled ever again
logonoff
force-pushed
the
OCPBUGS-66345-sdk-fix
branch
from
ef04d8a to
52d177c
Compare
|
/lgtm |
|
[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 |
|
/verified by api.spec.ts |
openshift-ci-robot
added
the
verified
|
@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. |
|
/label plugin-api-approved |
openshift-ci
bot
added
the
plugin-api-approved
|
/retest |
|
@logonoff: all tests passed! 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. |
|
@logonoff: Jira Issue Verification Checks: Jira Issue OCPBUGS-66345 Jira Issue OCPBUGS-66345 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in accepted release 4.21.0-0.nightly-2025-12-08-112148 |
7 participants
webpackimport usage withcompiler.webpackfor better rspack compatibility@openshift-console/dynamic-plugin-sdk-internal