Skip to content

Conversation

harsha509
Copy link
Member

@harsha509 harsha509 commented Oct 7, 2024

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

Remove npm and use only pnpm

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

enhancement, dependencies


Description

  • Updated several dependencies and devDependencies in package.json to their latest versions to ensure compatibility and leverage new features.
  • Updated Bazel dependencies in MODULE.bazel, specifically the rules_nodejs and esbuild toolchain versions, to maintain up-to-date build configurations.
  • Configured pnpm as the package manager in the root package.json, standardizing the package management approach across the project.

Changes walkthrough 📝

Relevant files
Dependencies
package.json
Update dependencies and devDependencies to latest versions

javascript/node/selenium-webdriver/package.json

  • Updated several dependencies to newer versions.
  • Updated several devDependencies to newer versions.
  • +9/-9     
    MODULE.bazel
    Update Bazel dependencies for NodeJS and esbuild                 

    MODULE.bazel

  • Updated rules_nodejs version from 6.2.0 to 6.3.0.
  • Updated esbuild toolchain version from 0.19.9 to 0.23.0.
  • +2/-2     
    Configuration changes
    package.json
    Set pnpm as the package manager                                                   

    package.json

    • Added pnpm as the package manager with version 9.6.0.
    +1/-0     
    Additional files (token-limit)
    pnpm-lock.yaml
    ...                                                                                                           

    pnpm-lock.yaml

    ...

    +757/-743

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

    @qodo-merge-pro qodo-merge-pro bot added B-dependencies Pull requests that update a dependency file P-enhancement PR with a new feature Review effort [1-5]: 2 labels Oct 7, 2024
    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 7, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Dependency Updates
    Multiple dependencies have been updated to newer versions. It's important to verify that these updates don't introduce breaking changes or compatibility issues.

    Build Configuration Change
    The Bazel configuration has been updated, including a version change for rules_nodejs and esbuild. This could potentially affect the build process and should be carefully reviewed.

    Package Manager Specification
    The package manager has been explicitly set to pnpm. This change should be reviewed to ensure it aligns with the project's package management strategy.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 7, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Specify the exact version of the package manager in the engines field for consistency

    Consider specifying the exact version of pnpm (9.6.0) in the engines field to ensure
    all developers use the same version.

    package.json [3]

     "packageManager": "[email protected]",
    +"engines": {
    +  "pnpm": "9.6.0"
    +},
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Specifying the exact version of the package manager in the engines field ensures all developers use the same version, which is important for consistency and avoiding version-related issues. This is a valuable improvement for the development process.

    8
    Use a more specific version range for dependencies to ensure consistent builds

    Consider using a more specific version range for the @bazel/runfiles dependency
    instead of ^6.3.0. This can help ensure consistent builds and prevent unexpected
    breaking changes.

    javascript/node/selenium-webdriver/package.json [26]

    -"@bazel/runfiles": "^6.3.0",
    +"@bazel/runfiles": "~6.3.0",
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using a more specific version range can help prevent unexpected breaking changes by ensuring consistent builds. This suggestion is relevant and can improve the stability of the project dependencies.

    7
    Maintainability
    Add comments to explain significant version updates in configuration files

    Consider adding a comment explaining the reason for updating the esbuild version to
    0.23.0, especially if there are specific features or improvements that are needed
    for the project.

    MODULE.bazel [79]

    +# Upgraded to esbuild 0.23.0 for improved performance and bug fixes
     esbuild.toolchain(esbuild_version = "0.23.0")
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Adding comments to explain version updates can improve maintainability by providing context for future developers. However, it is not critical for functionality, hence a moderate score.

    5

    💡 Need additional feedback ? start a PR chat

    @harsha509 harsha509 changed the base branch from trunk to js-npm-to-pnpm October 8, 2024 16:27
    @harsha509 harsha509 merged commit 0888197 into SeleniumHQ:js-npm-to-pnpm Oct 8, 2024
    9 of 11 checks passed
    harsha509 added a commit that referenced this pull request Oct 11, 2024
    * [js] use only pnpm and remove npm refs (#14572)
    
    * remove lock file refs
    M1troll pushed a commit to M1troll/selenium that referenced this pull request May 14, 2025
    * [js] use only pnpm and remove npm refs (SeleniumHQ#14572)
    
    * remove lock file refs
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    B-dependencies Pull requests that update a dependency file P-enhancement PR with a new feature Review effort [1-5]: 2

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant