Remove compare-versions dependency#1303
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the external compare-versions dependency by implementing a custom version comparison function in the project's utils module.
- Replaces the external
compare-versionspackage with a custom implementation - Updates import statements to use the new local function
- Removes the dependency from package.json
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Removes compare-versions dependency |
| lib/utils.js | Adds custom compareVersions function implementation |
| rosidl_parser/rosidl_parser.js | Updates import to use local compareVersions function |
| index.js | Updates import to use local compareVersions function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Parse version strings into arrays of integers | ||
| const v1Parts = version1.split('.').map((part) => parseInt(part, 10)); | ||
| const v2Parts = version2.split('.').map((part) => parseInt(part, 10)); |
There was a problem hiding this comment.
The parseInt function will return NaN for invalid version parts, which could lead to incorrect comparisons. Consider adding validation to ensure all parts are valid integers before proceeding with the comparison.
| // Parse version strings into arrays of integers | |
| const v1Parts = version1.split('.').map((part) => parseInt(part, 10)); | |
| const v2Parts = version2.split('.').map((part) => parseInt(part, 10)); | |
| // Helper to parse and validate version string | |
| function parseVersionParts(version) { | |
| const parts = version.split('.'); | |
| return parts.map((part, idx) => { | |
| // Only allow non-empty strings of digits | |
| if (!/^\d+$/.test(part)) { | |
| throw new Error(`Invalid version part '${part}' at position ${idx} in version string '${version}'`); | |
| } | |
| return parseInt(part, 10); | |
| }); | |
| } | |
| const v1Parts = parseVersionParts(version1); | |
| const v2Parts = parseVersionParts(version2); |
| * compareVersions('1.2.3', '1.2.3', '==') // true | ||
| * compareVersions('1.2.3', '1.2.3', '>=') // true | ||
| */ | ||
| function compareVersions(version1, version2, operator) { |
There was a problem hiding this comment.
The function doesn't validate input parameters. Consider adding checks for null/undefined version strings and ensuring the operator is a valid string to prevent runtime errors.
This PR removes the external `compare-versions` dependency by implementing a custom version comparison function in the project's utils module. - Replaces the external `compare-versions` package with a custom implementation - Updates import statements to use the new local function - Removes the dependency from package.json Fix: #1301
This PR removes the external
compare-versionsdependency by implementing a custom version comparison function in the project's utils module.compare-versionspackage with a custom implementationFix: #1301