V3 #18
V3 #18
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR represents a complete TypeScript rewrite of the tracking number validation library with substantial improvements and new features.
- Complete migration from JavaScript to TypeScript with full type definitions
- Addition of 5 new shipping carriers: LaserShip, Canada Post, China Post, Australia Post, and Royal Mail
- Implementation of tracking number generation capabilities for testing purposes
- Enhanced validation with advanced courier detection functions and checksum validation
- Updated UPS tracking URLs to remove deprecated
/mobile/path and added support for USPS barcodes starting with 95
Reviewed Changes
Copilot reviewed 33 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/validate_tracking_numbers.test.ts | New comprehensive TypeScript test suite replacing legacy JS tests |
| tests/tracking_numbers.ts | Updated test data with TypeScript definitions and extensive new carrier test cases |
| tests/is_valid.test.ts | New TypeScript validation tests with comprehensive invalid number coverage |
| tests/get_tracking_url.test.ts | New TypeScript URL generation tests with specific UPS URL fix validation |
| tests/get_couriers.test.ts | New TypeScript courier detection tests with enhanced error handling |
| src/index.ts | Complete TypeScript rewrite with modern API design and backward compatibility |
| src/generate-tracking.ts | New tracking number generation functionality with comprehensive format support |
| src/couriers/ | New modular TypeScript courier system with individual carrier modules |
| package.json | Updated dependencies, build system, and TypeScript tooling configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 33 out of 37 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| for (let i = digits.length - 2; i >= 0; i--) { | ||
| const digitChar = digits[i]; | ||
| if (!digitChar) continue; |
There was a problem hiding this comment.
The null check for digitChar is unnecessary since digits[i] will always return a string character or undefined. The check should be if (digitChar === undefined) continue; or simply removed since replace(/\D/g, '') ensures all characters are digits.
| if (!digitChar) continue; |
| } | ||
|
|
||
| const lastDigitChar = digits[digits.length - 1]; | ||
| if (!lastDigitChar) return false; |
There was a problem hiding this comment.
Similar to above, this null check is unnecessary. Since digits is created by replacing non-digits, lastDigitChar will always be a valid digit character or undefined if the string is empty (which is already handled by the length check at the beginning).
| if (!lastDigitChar) return false; |
| const sum = digits.slice(0, -1).split('').reduce((acc, d) => acc + parseInt(d), 0); | ||
| const checkDigit = sum % 7; | ||
| const lastDigitChar = digits[digits.length - 1]; | ||
| if (!lastDigitChar) return false; |
There was a problem hiding this comment.
The same unnecessary null check pattern appears here. This should be consistent with the validateMod10 function's handling.
| if (!lastDigitChar) return false; |
| const digits = match[1]; | ||
| const checkDigitStr = match[2]; | ||
|
|
||
| if (!digits || !checkDigitStr) return false; |
There was a problem hiding this comment.
The null check for digits and checkDigitStr is unnecessary since they are extracted from a successful regex match. If the regex matches, these capture groups will always contain strings.
| if (!digits || !checkDigitStr) return false; | |
| // The null check for digits and checkDigitStr is unnecessary since they are guaranteed by the regex match. |
| for (let i = 0; i < 8; i++) { | ||
| const digitChar = digits[i]; | ||
| const weight = weights[i]; | ||
| if (!digitChar || weight === undefined) return false; |
There was a problem hiding this comment.
The check for digitChar is unnecessary since digits comes from a regex match that ensures 8 digits. The weight check is also unnecessary since weights array has exactly 8 elements and i is bounded by the loop.
| if (!digitChar || weight === undefined) return false; |
Uh oh!
There was an error while loading. Please reload this page.