TSM-07: convert Group B services to TypeScript - #564
TSM-07: convert Group B services to TypeScript#564Justin Hammond (Justintime50) wants to merge 6 commits into
Conversation
175748d to
1e69a00
Compare
34e517c to
c192e9c
Compare
c192e9c to
6a8fb8c
Compare
6a8fb8c to
9facd28
Compare
9facd28 to
b0fa98d
Compare
Alex Matthews (ralexmatthews)
left a comment
There was a problem hiding this comment.
Nice. Non blocking, but just want clairification on the intention of these PRs. Are we explicitly wanting all the input types to be much more loosey-goosey than before, or are we trying to like recreate/reuse the well defined and structured types from before?
| type BatchCreateParameters = Record<string, unknown> & { | ||
| shipments?: Array<string | Record<string, unknown>> | null; | ||
| }; |
There was a problem hiding this comment.
thought:
These new types are still not as thorough as the old ones. For example, for the create batch params, here are the existing types in https://github.com/EasyPost/easypost-node/blob/master/types/Batch/BatchCreateParameters.d.ts:
export declare interface IBatchCreateParameters {
shipments?: Array<Shipment | string>;
}Where shipment is a fully fleshed type listing the fields and their types.
I'm wondering if the plan is to eventually go through and use these existing types or if they should be reimplemented here? We are already importing the batch type. I wonder if we should import the create param here as well?
| type PickupCreateParameters = Record<string, unknown> & { | ||
| address?: Record<string, unknown> | string | null; |
There was a problem hiding this comment.
thought:
Same thing here. This Record<string, unknown> is explicitly wider than what we had. Not sure if thats intentional. But then the sub-record fields are loosey goosey "just pass whatever", where the original types had
interface BasePickupCreateParameters {
address: Address | string;
// ...
Summary
.ts.Validation