Skip to content

Conversation

@avivtur
Copy link
Member

@avivtur avivtur commented Dec 30, 2025

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.6.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS. for mor info on this known issue from novnc: novnc/noVNC#1943
The workaround for that error was pinning the @novnc/novnc package to 1.5.0 which we already used instead of letting npm to upgrade to 1.6.0

🎥 Demo

Please add a video or an image of the behavior/changes

Summary by CodeRabbit

  • Chores
    • Pinned a third‑party library to an exact version for stability.
    • Clarified and standardized many type declarations across the codebase to improve type correctness and consistency (purely syntactic; no runtime changes).
    • Consolidated and cleaned up public exports and re-exports to remove duplicates and better organize exposed APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Dec 30, 2025

@avivtur: This pull request references CNV-74917 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 task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.5.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS.
The workaround for that error was downgrading the @novnc/novnc package to 1.4.0 where the top-level await doesn't exist.

🎥 Demo

Please add a video or an image of the behavior/changes

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 openshift-ci bot added the approved This issue is something we want to fix label Dec 30, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 30, 2025

📝 Walkthrough

Walkthrough

Pinned @novnc/novnc to exact 1.5.0 in package.json; added parentheses to several indexed-access TypeScript type expressions; expanded ConsoleComponentState with state and type fields; added/reordered re-exports and removed duplicates in self-validation utils.

Changes

Cohort / File(s) Change Summary
Dependency manifest
package.json
@novnc/novnc changed from ^1.5.0 to 1.5.0 (pinned exact version)
Type-expression parenthesization (multiple)
src/utils/components/Consoles/components/utils/types.ts, src/utils/components/Consoles/components/vnc-console/utils/util.ts, src/utils/components/NetworkIcons/utils.ts, src/utils/resources/instancetype/types.ts, src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts, src/views/checkups/utils/types.ts, src/views/clusteroverview/SettingsTab/tabs.ts
Adjusted indexed-access type expressions by adding parentheses (e.g., typeof X[Y](typeof X)[Y]) across several exported type aliases — syntactic/type-checking changes only
Console component state expansion
src/utils/components/Consoles/components/utils/types.ts
ConsoleComponentState type extended with two exported properties: state: ConsoleState and type: ConsoleType
Self-validation utils exports
src/views/checkups/self-validation/utils/index.ts
Added re-exports: downloadResults, getDefaultErrorMessage, validateDownloadInputs and types DownloadInputValidationResult, DownloadResultsReturn; moved PermissionOperationResult export; removed duplicate re-exports

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

lgtm

Suggested reviewers

  • rszwajko
  • Pedro-S-Abreu
  • adamviktora

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to 'rebuild node_modules and package-lock.json' but the actual primary change is downgrading @novnc/novnc to 1.4.0 due to a top-level await incompatibility with webpack 5. Update the title to accurately reflect the main change, such as 'CNV-74917: Downgrade @novnc/novnc to 1.4.0 for webpack 5 compatibility' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed PR description includes a clear reference to the Jira issue, explains the Node 22 upgrade context, details the @novnc/novnc compatibility issue and the downgrade rationale with a link to the upstream issue, but lacks the Demo section.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Dec 30, 2025

@avivtur: This pull request references CNV-74917 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 task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.5.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS.
The workaround for that error was downgrading the @novnc/novnc package to 1.4.0 where the top-level await doesn't exist.

🎥 Demo

Please add a video or an image of the behavior/changes

Summary by CodeRabbit

  • Chores
  • Adjusted a third-party library dependency version for enhanced compatibility and stability.

✏️ Tip: You can customize this high-level summary in your review settings.

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
Copy link
Collaborator

openshift-ci-robot commented Dec 30, 2025

@avivtur: This pull request references CNV-74917 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 task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.5.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS.
The workaround for that error was downgrading the @novnc/novnc package to 1.4.0 where the top-level await doesn't exist.

🎥 Demo

Please add a video or an image of the behavior/changes

Summary by CodeRabbit

  • Chores
  • Pinned a third‑party library to an exact version for stability.
  • Cleaned up and clarified type definitions and public exports across the codebase; these are syntactic/organizational changes with no user-facing behavior differences.

✏️ Tip: You can customize this high-level summary in your review settings.

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/views/checkups/self-validation/utils/index.ts (2)

11-14: LGTM – Export organization improvement.

Consolidating the download utilities and their types in a dedicated section improves clarity and maintainability. This reorganization does not change the public API surface.


44-45: LGTM – Logical export grouping.

Moving PermissionOperationResult after the permissions function exports creates better logical cohesion. This is a non-breaking refactor.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44e385f and 9dd2da7.

📒 Files selected for processing (8)
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/self-validation/utils/index.ts
  • src/views/checkups/utils/types.ts
  • src/views/clusteroverview/SettingsTab/tabs.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

Use .ts file extension for non-component files containing logic or utilities

Files:

  • src/views/clusteroverview/SettingsTab/tabs.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/utils/index.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{tsx,ts}: Extract logic from components into custom hooks or utility files to improve testability and component maintainability
Use React memoization tools (React.memo, useMemo, useCallback) to avoid unnecessary re-renders
Always specify dependencies in useEffect to avoid unnecessary re-renders or missed updates. Use an empty array [] if no dependencies are required

Files:

  • src/views/clusteroverview/SettingsTab/tabs.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/utils/index.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{ts,tsx,js,jsx}: Keep files under 150 lines whenever possible
Use descriptive names for variables, functions, and components. Avoid abbreviations unless widely recognized
Keep functions short and focused on one action. Apply Red → Green → Refactor methodology
Avoid hardcoded values (magic numbers) and define them as constants for easy adjustments and readability

Files:

  • src/views/clusteroverview/SettingsTab/tabs.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/utils/index.ts
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

Define constants in utility files with uppercase and underscore-separated naming (e.g., API_URL)

Files:

  • src/views/clusteroverview/SettingsTab/tabs.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/utils/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{ts,tsx}: Prefer using type instead of interface for defining the shapes of objects or functions in TypeScript
If a type is exported, add it to a utility file
Avoid using any type in TypeScript. Use unknown instead and narrow the type as needed
Always explicitly define return types for functions rather than relying on TypeScript type inference

Files:

  • src/views/clusteroverview/SettingsTab/tabs.ts
  • src/utils/components/NetworkIcons/utils.ts
  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
  • src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts
  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
  • src/views/checkups/self-validation/utils/index.ts
🧠 Learnings (5)
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{ts,tsx} : If a type is exported, add it to a utility file

Applied to files:

  • src/utils/components/Consoles/components/utils/types.ts
  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Avoid hardcoded values (magic numbers) and define them as constants for easy adjustments and readability

Applied to files:

  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{ts,tsx,js} : Define constants in utility files with uppercase and underscore-separated naming (e.g., `API_URL`)

Applied to files:

  • src/utils/components/Consoles/components/vnc-console/utils/util.ts
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{ts,tsx} : Avoid using `any` type in TypeScript. Use `unknown` instead and narrow the type as needed

Applied to files:

  • src/views/checkups/utils/types.ts
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{ts,tsx} : Prefer using `type` instead of `interface` for defining the shapes of objects or functions in TypeScript

Applied to files:

  • src/views/checkups/utils/types.ts
  • src/utils/resources/instancetype/types.ts
🧬 Code graph analysis (2)
src/utils/components/Consoles/components/utils/types.ts (1)
src/utils/components/Consoles/components/utils/ConsoleConsts.ts (1)
  • ConsoleTypes (27-31)
src/views/checkups/utils/types.ts (1)
src/views/checkups/utils/constants.ts (1)
  • CHECKUP_URLS (1-5)
🔇 Additional comments (8)
src/utils/resources/instancetype/types.ts (1)

25-25: LGTM – Type expression syntax update.

The parentheses around typeof InstanceTypeSizes make the operator precedence explicit. This is a safe, type-only change with no runtime impact.

src/utils/components/Consoles/components/vnc-console/utils/util.ts (1)

45-45: LGTM – Type expression syntax update.

The parenthesized form clarifies operator precedence. This syntax change is safe and consistent with the type updates across the codebase.

src/views/clusteroverview/SettingsTab/tabs.ts (1)

14-14: LGTM – Type expression syntax update.

Adding parentheses around the typeof expression is a safe syntactic clarification with no behavioral impact.

src/utils/components/NetworkIcons/utils.ts (1)

19-22: LGTM – Type expression syntax update.

The parenthesized return type follows the same explicit operator precedence pattern applied throughout the codebase. No behavioral change.

src/views/checkups/utils/types.ts (1)

11-11: LGTM – Type expression syntax update.

The parenthesized indexed access type is syntactically clearer and consistent with the broader pattern applied across the PR.

src/views/checkups/self-validation/components/actions/CheckupsSelfValidationActionsUtils.ts (1)

10-11: LGTM – Type expression syntax update.

This completes the consistent pattern of parenthesizing typeof expressions before indexed access across the codebase. Safe and clear.

src/utils/components/Consoles/components/utils/types.ts (2)

5-5: LGTM – Type expression syntax update.

The parenthesized form is consistent with the type syntax updates throughout the codebase.


7-11: No breaking changes detected—type expansion is already practiced in the codebase.

Both construction sites (Consoles.tsx and VirtualMachinesOverviewTabDetailsConsole.tsx) already initialize ConsoleComponentState with all three fields (actions, state, and type). The type expansion formalizes the existing object shape that the code already uses. Producer functions employ Partial<ConsoleComponentState> for flexible updates, ensuring compatibility with incremental state changes.

Signed-off-by: Aviv Turgeman <[email protected]>
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Jan 4, 2026

@avivtur: This pull request references CNV-74917 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 task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.5.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS.
The workaround for that error was downgrading the @novnc/novnc package to 1.4.0 where the top-level await doesn't exist.

🎥 Demo

Please add a video or an image of the behavior/changes

Summary by CodeRabbit

  • Chores
  • Pinned a third‑party library to an exact version for stability.
  • Clarified and standardized many type declarations across the codebase to improve type correctness and consistency (purely syntactic; no runtime changes).
  • Consolidated and cleaned up public exports and re-exports to remove duplicates and better organize exposed APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

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.

@avivtur avivtur changed the title CNV-74917: downgrade @novnc/novnc to 1.4.0 CNV-74917: rebuild node_modules and package-lock.json Jan 4, 2026
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Jan 4, 2026

@avivtur: This pull request references CNV-74917 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 task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

https://issues.redhat.com/browse/CNV-74917

This PR is rebuilding the package-lock.json + node_modules/ files from scratch to upgrade our midstream to use node 22 instead of node 20 as it blocks the builds.
After regenerating the package-lock.json + node_modules and building, I had an error with @novnc/novnc package.
version 1.6.0 of @novnc/novnc introduces a top-level await (await outside of a function) which is forbidden by webpack 5 for commonJS. for mor info on this known issue from novnc: novnc/noVNC#1943
The workaround for that error was pinning the @novnc/novnc package to 1.5.0 which we already used instead of letting npm to upgrade to 1.6.0

🎥 Demo

Please add a video or an image of the behavior/changes

Summary by CodeRabbit

  • Chores
  • Pinned a third‑party library to an exact version for stability.
  • Clarified and standardized many type declarations across the codebase to improve type correctness and consistency (purely syntactic; no runtime changes).
  • Consolidated and cleaned up public exports and re-exports to remove duplicates and better organize exposed APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

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 openshift-ci bot added the lgtm Passed code review, ready for merge label Jan 4, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 4, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avivtur, metalice

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 3681b7e into kubevirt-ui:main Jan 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved This issue is something we want to fix jira/valid-reference lgtm Passed code review, ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants