Skip to content

TSM-06: convert Group A services to TypeScript - #563

Open
Justin Hammond (Justintime50) wants to merge 14 commits into
ts-migrate/05-base-service-hydrationfrom
ts-migrate/06-services-group-a
Open

TSM-06: convert Group A services to TypeScript#563
Justin Hammond (Justintime50) wants to merge 14 commits into
ts-migrate/05-base-service-hydrationfrom
ts-migrate/06-services-group-a

Conversation

@Justintime50

@Justintime50 Justin Hammond (Justintime50) commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

Converts Group A service modules to TypeScript and applies minimal typed compatibility fixes needed for the current base-service conversion, with runtime behavior preserved.

Changes in this PR

  • renames Group A service files to .ts:
    • src/services/address_service.ts
    • src/services/parcel_service.ts
    • src/services/customs_info_service.ts
    • src/services/customs_item_service.ts
    • src/services/shipment_service.ts
  • adds minimal type-safe wrapper map annotations where property writes are dynamic
  • updates conversion helper callsites to pass params consistently with current base service signatures
  • introduces no new ts-nocheck

Testing

  • npm run typescript
  • npm run build
  • npx eslint src/services/address_service.ts src/services/parcel_service.ts src/services/customs_info_service.ts src/services/customs_item_service.ts src/services/shipment_service.ts
  • npx vitest run test/services/address.test.js test/services/parcel.test.js test/services/customs_info.test.js test/services/customs_item.test.js test/services/shipment.test.js test/services/module_exports_compat.test.js

Notes

  • targeted service suites above are blocked locally without EASYPOST_TEST_API_KEY; module export compatibility test is included in the command set.

@Justintime50 Justin Hammond (Justintime50) changed the title step6: convert group a services to ts TSM-06: convert Group A services to TypeScript Aug 6, 2026
@Justintime50
Justin Hammond (Justintime50) force-pushed the ts-migrate/06-services-group-a branch 3 times, most recently from ea893f6 to 66869ff Compare August 7, 2026 19:27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... this looks like its more of a "just adding types for the sake of adding types" and less like implementing typescript checking. Lots of unknowns, anys, and general type omissions. I see lots of places where we introduce more any's, and while we technically have "explicit parameter and return types", I don't think most of them are actually helpful. So many are just

(params: Record<string, unknown>): Promise<unknown>
Image

I might not be getting the point of this, and they are typed in the ./types/ directory. I don't know if we want to implement those in the source directly instead, or treat those as the source of truth or what, but it feels divergent.

const url = 'addresses';

const wrappedParams = {};
const wrappedParams: Record<string, unknown> = {};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
Looks like this is a known type, and could probably strengthen it just a little bit.

type MetaAddressKeys = "verify" | "verify_strict" | "verify_carrier";
type AddressCreateInternalParameters = 
  { address?: Omit<AddressCreateParameters, MetaAddressKeys> } & 
  Pick<AddressCreateParameters, MetaAddressKeys>;

Comment thread src/services/address_service.ts Outdated
* @returns {Object} - An object containing a list of {@link Address addresses} and pagination information.
*/
static async all(params = {}) {
static async all(params: Record<string, unknown> = {}): Promise<unknown> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
Do we really not know what these return? It feels like these should be known.

@Justintime50
Justin Hammond (Justintime50) marked this pull request as ready for review August 12, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants