Skip to content

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Jan 2, 2025

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Fix the error from #14993

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Dependencies


Description

  • Fixed dependency issue with @novnc/novnc in the grid UI.

  • Updated Bazel build configuration to include @novnc/novnc/core/rfb.


Changes walkthrough 📝

Relevant files
Dependencies
BUILD.bazel
Update Bazel build file for `novnc` dependency                     

javascript/grid-ui/BUILD.bazel

  • Added @novnc/novnc/core/rfb to the external dependencies list.
  • Ensured compatibility with the updated novnc version.
  • +1/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 2, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Move package dependency from external exclusions list to dependencies section to ensure proper bundling

    The external dependency @novnc/novnc/core/rfb should be added to the deps section
    rather than the external section, as it's a direct package dependency that needs to
    be bundled, not excluded.

    javascript/grid-ui/BUILD.bazel [76-83]

     external = [
         "fs",
         "module",
         "os",
         "path",
         "util",
    -    "@novnc/novnc/core/rfb",
     ],
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a critical configuration error. The external list is for excluding modules from bundling, while @novnc/novnc/core/rfb needs to be included in the bundle, so it should be in the deps section instead.

    9

    @VietND96 VietND96 merged commit 3a8395b into trunk Jan 2, 2025
    11 checks passed
    @VietND96 VietND96 deleted the grid-deps branch January 2, 2025 09:42
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants