OCPBUGS-88319, CONSOLE-5355: Allow customizing skipImportPrefixes when building Console plugins and begin adoption of api types package#16585
Conversation
|
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:
WalkthroughThis PR makes dynamic-module import skip prefixes configurable end-to-end: ConsoleRemotePlugin accepts caller prefixes, DynamicModuleImportPlugin merges them with built-in PatternFly defaults and forwards the deduped list to the loader, and the loader now computes JSX resourceMetadata from the resource path by default. ChangesImport-skip configuration refactoring
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
component/sdk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts`:
- Around line 210-217: Add JSDoc for the exported function
getDynamicModuleImportSkipPrefixes explaining its purpose (returns an array of
module path prefixes that should be skipped during dynamic module import
resolution), describe the return value (unique array combining default skip
prefixes and any provided additionalPrefixes), list what the default prefixes
cover (PatternFly deprecated/internal ESM paths such as
'`@patternfly/react-core/deprecated`' and specific internal
Tooltip/Popover/createIcon paths), and document how additionalPrefixes are
merged (appended to defaults and deduplicated via _.uniq). Include a short usage
example showing calling getDynamicModuleImportSkipPrefixes() with no args and
with a custom array, and mention the parameter name and type
(additionalPrefixes: string[] = []).
In
`@frontend/packages/console-dynamic-plugin-sdk/src/webpack/DynamicModuleImportPlugin.ts`:
- Around line 136-139: The options type currently requires skipImportPrefixes
which breaks public API; change the DynamicModuleImportPluginOptions type so
skipImportPrefixes is optional (skipImportPrefixes?: string[]) and ensure the
DynamicModuleImportPlugin constructor or initialization uses a default empty
array ([]) when options.skipImportPrefixes is undefined; update references
inside DynamicModuleImportPlugin to rely on the defaulted value so external
callers can omit this field.
🪄 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: ff15f541-54ff-464f-917e-3beef4d2bbc8
📒 Files selected for processing (3)
frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.tsfrontend/packages/console-dynamic-plugin-sdk/src/webpack/DynamicModuleImportPlugin.tsfrontend/webpack.config.ts
| export const getDynamicModuleImportSkipPrefixes = (additionalPrefixes: string[] = []) => | ||
| _.uniq([ | ||
| '@patternfly/react-core/deprecated', | ||
| '@patternfly/react-icons/dist/esm/createIcon', | ||
| '@patternfly/react-core/dist/esm/components/Tooltip/', | ||
| '@patternfly/react-core/dist/esm/components/Popover/', | ||
| ...additionalPrefixes, | ||
| ]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document this public API function.
getDynamicModuleImportSkipPrefixes is exported from the console-dynamic-plugin-sdk package, making it part of the public API. External plugins may use this function to configure their skip prefixes. Add JSDoc documentation explaining:
- Purpose and return value
- What the default prefixes cover (deprecated/internal PatternFly paths)
- How additionalPrefixes are merged with defaults
📝 Suggested JSDoc
+/**
+ * Get the list of import prefixes to skip during dynamic module import transformation.
+ *
+ * This function combines a default list of PatternFly-specific paths (deprecated/internal modules)
+ * with optional caller-provided prefixes. The result is deduplicated.
+ *
+ * `@param` additionalPrefixes - Optional array of additional import prefixes to skip.
+ * `@returns` Deduplicated array of import prefixes to skip.
+ *
+ * `@example`
+ * ```ts
+ * // Use default skip prefixes only
+ * const prefixes = getDynamicModuleImportSkipPrefixes();
+ *
+ * // Add custom skip prefixes
+ * const prefixes = getDynamicModuleImportSkipPrefixes([
+ * '`@patternfly/react-core/dist/esm/components/OverflowMenu/OverflowMenuContext`',
+ * ]);
+ * ```
+ */
export const getDynamicModuleImportSkipPrefixes = (additionalPrefixes: string[] = []) =>🤖 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/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts`
around lines 210 - 217, Add JSDoc for the exported function
getDynamicModuleImportSkipPrefixes explaining its purpose (returns an array of
module path prefixes that should be skipped during dynamic module import
resolution), describe the return value (unique array combining default skip
prefixes and any provided additionalPrefixes), list what the default prefixes
cover (PatternFly deprecated/internal ESM paths such as
'`@patternfly/react-core/deprecated`' and specific internal
Tooltip/Popover/createIcon paths), and document how additionalPrefixes are
merged (appended to defaults and deduplicated via _.uniq). Include a short usage
example showing calling getDynamicModuleImportSkipPrefixes() with no args and
with a custom array, and mention the parameter name and type
(additionalPrefixes: string[] = []).
|
/retitle OCPBUGS-88319: Allow customizing skipImportPrefixes when building Console plugins |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@vojtechszocs: This pull request references Jira Issue OCPBUGS-88319, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
from
19a65e7 to
dad0a2e
Compare
|
@vojtechszocs: This pull request references Jira Issue OCPBUGS-88319, which is valid. 3 validation(s) were run on this bug
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
force-pushed
the
plugin-import-transform-skip-customization
branch
2 times, most recently
from
be36d09 to
29d153d
Compare
| For older 1.x plugin SDK packages, refer to "OpenShift Console Versions vs SDK Versions" compatibility | ||
| table in [Console dynamic plugins README](./README.md). | ||
|
|
||
| ## 4.23.0-prerelease.3 - TBD |
There was a problem hiding this comment.
Skipped prerelease.2 because 4.23.0-prerelease.2 is out for lib-core. Better to avoid confusion
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md (1)
151-151:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing link references for OCPBUGS-88319 and
#16585.Once line 15 is corrected, the changelog will need link references for
OCPBUGS-88319and#16585. Add them to the link reference section following the existing pattern.📝 Proposed additions
Add after line 151 in the OCPBUGS section:
[OCPBUGS-84338]: https://issues.redhat.com/browse/OCPBUGS-84338 +[OCPBUGS-88319]: https://issues.redhat.com/browse/OCPBUGS-88319 [`#13188`]: https://github.com/openshift/console/pull/13188Add after line 178 in the PR reference section:
[`#16376`]: https://github.com/openshift/console/pull/16376 +[`#16585`]: https://github.com/openshift/console/pull/16585🤖 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/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md` at line 151, Add missing Markdown link reference entries for OCPBUGS-88319 and `#16585` in the changelog's link reference section, following the existing pattern used by the OCPBUGS and PR references (e.g., mirror the format of the existing "[OCPBUGS-84338]: https://issues.redhat.com/browse/OCPBUGS-84338" line and the PR reference entries), so add a "[OCPBUGS-88319]: <JIRA-URL>" entry in the OCPBUGS references and a "[`#16585`]: <PR-URL>" entry in the PR reference section using the correct upstream URLs for those identifiers.
🤖 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/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md`:
- Line 15: Update the changelog entry to correct typos and references and make
the feature description precise: change "surpress" to "suppress", replace JIRA
reference CONSOLE-5065 with OCPBUGS-88319 and PR reference `#16224` with `#16585`,
and reword the description to state that DynamicModuleImportLoaderOptions allows
customizing skipImportPrefixes to suppress dynamic module import warnings
(rather than a general "extension of imports"); ensure the entry mentions the
DynamicModuleImportLoaderOptions type and skipImportPrefixes explicitly.
---
Outside diff comments:
In `@frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md`:
- Line 151: Add missing Markdown link reference entries for OCPBUGS-88319 and
`#16585` in the changelog's link reference section, following the existing pattern
used by the OCPBUGS and PR references (e.g., mirror the format of the existing
"[OCPBUGS-84338]: https://issues.redhat.com/browse/OCPBUGS-84338" line and the
PR reference entries), so add a "[OCPBUGS-88319]: <JIRA-URL>" entry in the
OCPBUGS references and a "[`#16585`]: <PR-URL>" entry in the PR reference section
using the correct upstream URLs for those identifiers.
🪄 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: 18600ce7-15f8-49ec-acfc-cf5e6e95fd7e
📒 Files selected for processing (4)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.tsfrontend/packages/console-dynamic-plugin-sdk/src/webpack/DynamicModuleImportPlugin.tsfrontend/packages/console-dynamic-plugin-sdk/src/webpack/loaders/dynamic-module-import-loader.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
4 times, most recently
from
4a3adfb to
c4b2123
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/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.ts`:
- Line 479: The forwarded value for skipImportPrefixes from
sharedDynamicModuleSettings can be undefined and causes a runtime spread error
in DynamicModuleImportPlugin.apply; in ConsoleRemotePlugin where you pass
skipImportPrefixes: sharedDynamicModuleSettings.skipImportPrefixes, default it
to an empty array (e.g. use sharedDynamicModuleSettings.skipImportPrefixes ??
[]) so DynamicModuleImportPlugin receives an iterable; update the forwarding
site in ConsoleRemotePlugin (the skipImportPrefixes property) to ensure a safe
default before calling DynamicModuleImportPlugin.
🪄 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: 423fc980-3361-4b8d-8d1c-b281b7cf22b8
📒 Files selected for processing (3)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/console-dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin.tsfrontend/packages/console-dynamic-plugin-sdk/src/webpack/DynamicModuleImportPlugin.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md
- frontend/packages/console-dynamic-plugin-sdk/src/webpack/DynamicModuleImportPlugin.ts
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
2 times, most recently
from
1602b57 to
569b7f0
Compare
Replace the webpack-only `NormalModule.beforeLoaders` hook with a standard module rule. The loader now derives the `jsx` flag from `this.resourcePath` instead of receiving it per-module via hook-injected options. This is also supported by rspack while the previous impl. was not.
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
from
569b7f0 to
df61270
Compare
openshift-ci
Bot
changed the title
|
@vojtechszocs: This pull request references Jira Issue OCPBUGS-88319, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. This pull request references CONSOLE-5355 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. |
logonoff
left a comment
logonoff
left a comment
There was a problem hiding this comment.
/lgtm
/label plugin-api-approved
openshift-ci
Bot
added
plugin-api-approved
|
build infra only changes |
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. |
Replace local K8sResourceCommon-based type definitions with the official OpenShift API types. Import ConsoleCLIDownloadKind and ConsoleNotificationKind from @openshift/api-types and update useK8sWatchResource generics and props to use those types. Removes now-unused local CLIDownload and ConsoleNotification type declarations and cleans up imports. No functional changes intended; this aligns types with upstream API definitions.
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
from
a58aa2a to
d66f375
Compare
openshift-ci-robot
removed
the
verified
openshift-ci
Bot
added
kind/demo-plugin
|
/lgtm |
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. |
Replace hand-written K8sResourceCommon type definitions with re-exports from `@openshift/api-types` where structurally compatible. This covers 10 Kind types in `module/k8s/types.ts` and the vsphere Secret type. Intersect shipwright `@kubernetes-models` types with K8sResourceCommon to bridge metadata type incompatibility between `IObjectMeta` and the `ObjectMetadata` now re-exported from `@openshift/api-types`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
logonoff
force-pushed
the
plugin-import-transform-skip-customization
branch
from
d66f375 to
1731bc8
Compare
openshift-ci-robot
removed
the
verified
|
/lgtm |
|
@vojtechszocs: 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. |
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. |
|
[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 |
1 similar comment
|
[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 |
5 participants
Analysis / Root cause
When building Console with webpack, the following warning is emitted by
DynamicModuleImportPluginThis is caused by
@patternfly/react-corepackage not exposingOverflowMenuContextdirectly via package indexso the relevant import in
ResponsiveActionDropdown.tsxcannot be transformed into something likewhich means
OverflowMenuContextcode will not be shared with other Console plugins via suchdist/dynamicmodule.To fix this particular issue in Console as well as any similar issues in Console plugins - maintainers of vendor packages that support dynamic modules such as
@patternfly/react-coreshould add the relevant export and ensure thatdist/dynamic-modules.jsonhas relevant entries likeSolution description
Allow Console and its dynamic plugins to customize
skipImportPrefixesas a workaround to reduce build warning noise for known issues related to dynamic module import processing.Test cases
OverflowMenuContextReviewers and assignees
/assign @logonoff
Summary by CodeRabbit
New Features
Documentation