Remove ts-strict-ignore and fix TypeScript errors [shipping]#6114
Remove ts-strict-ignore and fix TypeScript errors [shipping]#6114lkostrowski wants to merge 4 commits intomainfrom
Conversation
…rrors Removed @ts-strict-ignore comments from all 17 files in src/shipping directory and fixed TypeScript strict mode errors by adding proper null/undefined checks and type annotations. Changes: - Added null/undefined safety checks throughout the codebase - Fixed function return types to handle undefined cases - Added proper type annotations for previously untyped parameters - Converted null checks to handle both null and undefined (using == null) - Added runtime checks before accessing potentially undefined properties Reduced TypeScript strict errors from 91 to 62 (32% reduction). Remaining errors follow similar patterns and will be addressed in follow-up. Test results: All existing tests pass.
|
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6114 +/- ##
==========================================
- Coverage 39.93% 39.91% -0.03%
==========================================
Files 2419 2419
Lines 40017 40040 +23
Branches 8819 9166 +347
==========================================
Hits 15980 15980
- Misses 24008 24031 +23
Partials 29 29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request removes TypeScript strict mode ignore comments from 17 files in the src/shipping directory and adds necessary type safety improvements including null/undefined checks, proper type annotations, and error handling enhancements. The changes reduce TypeScript strict errors from 91 to 62, representing a 32% improvement in type safety.
Key changes:
- Added null/undefined safety checks using optional chaining and fallback values throughout shipping components and handlers
- Introduced proper type annotations for previously untyped function parameters (e.g., PostalCodeRule interface)
- Enhanced error handling by adding guard clauses and fallback return values
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shipping/views/utils.tsx | Added PostalCodeRule interface and type annotations for filterPostalCodes and getPostalCodeRuleByMinMax functions |
| src/shipping/views/ShippingZoneDetails/index.tsx | Removed ts-strict-ignore comment only |
| src/shipping/views/ShippingZoneCreate.tsx | Removed ts-strict-ignore comment only |
| src/shipping/views/RateCreate.tsx | Removed ts-strict-ignore comment only |
| src/shipping/handlers.ts | Added null checks for rules, description fields, and addPostalCodeRules; added error throwing logic for missing data; improved optional chaining for error access |
| src/shipping/errors.ts | Added undefined check for error parameter and ensured string return type with fallback |
| src/shipping/components/ShippingZoneRatesCreatePage/ShippingZoneRatesCreatePage.tsx | Added fallback value (0) for allChannelsCount prop |
| src/shipping/components/ShippingZoneRates/ShippingZoneRates.tsx | Removed ts-strict-ignore comment only |
| src/shipping/components/ShippingZonePostalCodes/ShippingZonePostalCodes.tsx | Added type annotation for inclusionType state, null checks for postalCodes, explicit type definition for postalCodeRange parameter, and null-to-undefined conversion for renderCollection |
| src/shipping/components/ShippingZoneDetailsPage/utils.ts | Added fallback empty arrays for metadata, privateMetadata, and channels mapping |
| src/shipping/components/ShippingZoneDetailsPage/ShippingZoneDetailsPage.tsx | Removed ts-strict-ignore comment only |
| src/shipping/components/ShippingZoneCreatePage/ShippingZoneCreatePage.tsx | Removed ts-strict-ignore comment only |
| src/shipping/components/ShippingZoneCountriesAssignDialog/handlers.ts | Removed ts-strict-ignore comment only |
| src/shipping/components/ShippingRateInfo/ShippingRateInfo.tsx | Removed ts-strict-ignore comment only |
| src/shipping/components/ShippingMethodProducts/ShippingMethodProducts.tsx | Removed ts-strict-ignore comment only |
| src/shipping/components/PricingCard/PricingCard.tsx | Added null safety by mapping errors to ensure channels field is always an array |
| src/shipping/components/OrderWeight/OrderWeight.tsx | Removed ts-strict-ignore comment only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!response.data?.shippingPriceCreate) { | ||
| throw new Error("Failed to create shipping rate"); | ||
| } |
There was a problem hiding this comment.
Throwing an error here will cause an unhandled exception. The function createShippingRate is called through useHandleFormSubmit, which doesn't have try-catch error handling. This will crash the application instead of showing a user-friendly error. Consider returning an error array consistent with the pattern used when createErrors.length > 0 (line 232-234), or add proper error handling in the call chain.
| if (!response.data.shippingPriceCreate.shippingMethod) { | ||
| throw new Error("Failed to create shipping method"); | ||
| } |
There was a problem hiding this comment.
Throwing an error here will cause an unhandled exception. The function createShippingRate is called through useHandleFormSubmit, which doesn't have try-catch error handling. This will crash the application instead of showing a user-friendly error. Consider returning an error array consistent with the pattern used elsewhere in this function, or add proper error handling in the call chain.
- Fixed PostalCodeRule type to derive from GraphQL fragment type - Added proper null/undefined checks in RateCreate and RateUpdate - Fixed type imports for ShippingMethodWithPostalCodesFragment - Reduced strict mode errors from 91 to 53 (42% reduction) Remaining errors are primarily null checks in components and views.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <TableRowLink> | ||
| <TableCell> | ||
| {postalCodes === undefined ? ( | ||
| {postalCodes == null ? ( |
There was a problem hiding this comment.
[nitpick] Inconsistent spacing in null check. The comparison operator should be == null (without extra space) for consistency with typical JavaScript/TypeScript style. While this doesn't affect functionality, it's inconsistent with standard formatting.
| {postalCodes == null ? ( | |
| {postalCodes==null ? ( |
| postalCodeRules: filterPostalCodes(state.postalCodeRules, code), | ||
| postalCodeRules: filterPostalCodes( | ||
| state.postalCodeRules!, | ||
| code as NonNullable<ShippingMethodWithPostalCodesFragment["postalCodeRules"]>[number], |
There was a problem hiding this comment.
The type assertion here is redundant. The function filterPostalCodes already expects and returns the correct type PostalCodeRule[]. The code parameter can be passed directly without the type assertion:
postalCodeRules: filterPostalCodes(state.postalCodeRules!, code),| code as NonNullable<ShippingMethodWithPostalCodesFragment["postalCodeRules"]>[number], | |
| code, |
| params.type!, | ||
| state.postalCodeRules || [], | ||
| state.inclusionType!, |
There was a problem hiding this comment.
Using a non-null assertion (params.type!) on optional URL parameters is unsafe. If params.type is undefined, this will cause a runtime error. Consider adding proper validation:
if (!params.type) {
// Handle missing type parameter
return null; // or navigate back
}
const { channelErrors, createShippingRate, errors, status } = useShippingRateCreator(
id,
params.type,
state.postalCodeRules || [],
state.inclusionType!,
);| } | ||
|
|
||
| const newCode = getRuleObject(rule, state.inclusionType); | ||
| const newCode = getRuleObject(rule, state.inclusionType!); |
There was a problem hiding this comment.
Using a non-null assertion (state.inclusionType!) is unsafe. The inclusionType in the reducer state is marked as optional (inclusionType?: PostalCodeRuleInclusionTypeEnum). While it's initialized with a default value in line 68, the state could potentially be updated with undefined. Consider using nullish coalescing for safety:
const newCode = getRuleObject(rule, state.inclusionType ?? PostalCodeRuleInclusionTypeEnum.EXCLUDE);| const newCode = getRuleObject(rule, state.inclusionType!); | |
| const newCode = getRuleObject(rule, state.inclusionType ?? PostalCodeRuleInclusionTypeEnum.EXCLUDE); |
| __typename: "ShippingMethodPostalCodeRule" as const, | ||
| end: rule.max, | ||
| id: undefined, | ||
| id: "0", |
There was a problem hiding this comment.
Using a hardcoded "0" as the default id could cause issues. When postal codes are created locally (before being saved to the backend), they're assigned id "0". If multiple postal codes are created in succession, they'll all have the same id, which could cause problems with React's key prop or duplicate detection logic. Consider using a more unique identifier like:
id: `temp-${Date.now()}-${Math.random()}`,or a counter-based approach.
| id: "0", | |
| id: `temp-${Date.now()}-${Math.random()}`, |
| const formErrors = getFormChannelErrors(["price"], errors); | ||
| const formErrors = getFormChannelErrors( | ||
| ["price"], | ||
| errors.map(err => ({ ...err, channels: err.channels || [] })), |
There was a problem hiding this comment.
While this mapping ensures channels is always defined, creating a new array of error objects with spread operations for every render is inefficient. Since ShippingChannelsErrorFragment should guarantee channels is defined (based on GraphQL schema), a type assertion would be more efficient. If the schema truly allows nullable channels, consider memoizing this transformation with useMemo:
const formErrors = useMemo(
() => getFormChannelErrors(
["price"],
errors.map(err => ({ ...err, channels: err.channels || [] })),
),
[errors]
);| errors.map(err => ({ ...err, channels: err.channels || [] })), | |
| errors as (ShippingChannelsErrorFragment & { channels: NonNullable<ShippingChannelsErrorFragment["channels"]> })[], |
3 participants
…rrors
Removed @ts-strict-ignore comments from all 17 files in src/shipping directory and fixed TypeScript strict mode errors by adding proper null/undefined checks and type annotations.
Changes:
Reduced TypeScript strict errors from 91 to 62 (32% reduction). Remaining errors follow similar patterns and will be addressed in follow-up.
Test results: All existing tests pass.
Scope of the change