Shopify-api-node icon indicating copy to clipboard operation
Shopify-api-node copied to clipboard

Type definition improvements.

Open YourWishes opened this issue 5 years ago • 5 comments

I've been working with the API and I have a few ideas for type definition improvements, I'll submit a few pull requests when I can find some time as I've made a few of these already.

Fixes

  • ~ICarrierService - Needs id:number as part of the interface~
  • ICreateArticle - image is only assignable to type of IBase64Image, when IShopifyImage could also be accepted.
  • ICheckoutDiscount - Missing optional code string.

Improvements to types

  • ICollection - Interface for a standard collection model, since ISmartCollection and ICustomCollection have a fair bit of overlap, also the various collection webhooks e.g. /collections/create return essentially either ISmartCollection | ICustomCollection
  • Better use of params for various functions, e.g. product.list currently accepts params?:any
  • Better use of similar object types. e.g. ILineItemProperty vs ICheckoutLineItemProperty, essentially doing the same thing.

Additions

  • ~WebhookRequestMap - Interface for lookup of WebhookTopic to an interface of webhook request. e.g. WebhookTopic products/create can have request of IProduct. This will require additions to what can be given by a webhook, e.g.~
    • ~ICart - Interface for cart webhook callbacks~
    • ~IDeleteItem - Interface for a webhook callback for deleted item, generally { id:number }~
    • ICollection - Refer to improvements to types
  • ~ICarrierRequest - Interface for carrier request callbacks, will also require ICarrierItem and ICarrierAddress~
  • ~ICarrierResponse - Interface for carrier responses to Shopify~
  • RequestError (or similar) - Enumerator of errors that Shopify may return.
  • IOrderLineItem - Seems to be missing a bit of stuff, particularly discount_applications
  • AccessScopes - Enumerator of possible access scopes, also useful for accessScope.list
    • This could potentially lead to having some form of precheck for available access scopes and giving us type definitions depending on access scope availibility.

Improvements to structure

  • At the moment all the types are defined in the master index.d.ts file, it can be a bit much to navigate, perhaps a better defined folder structure would help with additions and changes.

I guess there's also a case to be made for converting the project from Javascript to Typescript, if this is something that is being considered.

YourWishes avatar Jun 22 '19 00:06 YourWishes

I was going to create an issue related to Pagination, but this seems like the right place to add.

My static type checker is unhappy with order.nextPageParameters as the response is defined as IOrder[]. I'm not super knowledgeable in typescript, but I think maybe something like:

interface IOrderList extends Array<IOrder> {
  nextPageParameters?: string,
  previousPageParameters?: string,
}

Alternatively maybe a response interface that could contain any of the typed array responses so as not to need to add a list type for every endpoint with a list api call?

DaveLo avatar Nov 26 '19 16:11 DaveLo

I had a similar issue with paginated results on products. Something as simple as a generic this

interface IPaginatedResult<T> extends Array<T> {
		nextPageParameters?: any;
		previousPageParameters?: any,
}

Then change the return types for all paginated responses (not all are currently implemented according to the docs as of today).

order: {
    cancel: (id: number, params?: any) => Promise<Shopify.IOrder>;
    close: (id: number) => Promise<Shopify.IOrder>;
    count: (params?: any) => Promise<number>;
    create: (params: any) => Promise<Shopify.IOrder>;
    delete: (id: number) => Promise<any>;
    get: (id: number, params?: any) => Promise<Shopify.IOrder>;
    list: (params?: any) => Promise<IPaginatedResult<Shopify.IOrder>>;
    open: (id: number) => Promise<Shopify.IOrder>;
    update: (id: number, params: any) => Promise<Shopify.IOrder>;
  };

product: {
    count: (params?: any) => Promise<number>;
    create: (params: any) => Promise<Shopify.IProduct>;
    delete: (id: number) => Promise<void>;
    get: (id: number, params?: any) => Promise<Shopify.IProduct>;
    list: (params?: any) => Promise<IPaginatedResult<Shopify.IProduct>>;
    update: (id: number, params: any) => Promise<Shopify.IProduct>;
  };

In addition there is currently no support in the types for bigInt. It appears as though you are handling sending and retrieving the data with json-bigint but the types sending data are defined as number which will be a problem in the near future. A native bigint type was added in Typscript 3.2 but targets the native esnext bigint type and would not be compatible with the bigInt library used currently.

 fulfillment: {
    cancel: (orderId: bigint, id: number) => Promise<Shopify.IFulfillment>;
    complete: (orderId: bigint, id: number) => Promise<Shopify.IFulfillment>;
    count: (orderId: bigint, params?: any) => Promise<number>;
    create: (orderId: bigint, params: any) => Promise<Shopify.IFulfillment>;
    get: (
      orderId: bigint,
      id: number,
      params?: any
    ) => Promise<Shopify.IFulfillment>;
    list: (orderId: bigint, params?: any) => Promise<Shopify.IFulfillment[]>;
    open: (orderId: bigint, id: number) => Promise<Shopify.IFulfillment>;
    update: (
      orderId: bigint,
      id: number,
      params: any
    ) => Promise<Shopify.IFulfillment>;
  };

bags1976 avatar Nov 29 '19 21:11 bags1976

I think it could be beneficial to have something similar to;

type IPaginatedResults<T> = T & {
  nextPageParameters?:T & IPaginatedResults<T>;
  previousPageParameters?:T & IPaginatedResults<T>;
}

//Then:
let response = order.list({ limit: 50 });
type X = typeof response;//Shopify.IOrder & IPaginatedResults<{ limit:number }>;

I assume the TS Compiler will struggle with this recursive type definition a bit, but something similar came to mind.

YourWishes avatar Nov 30 '19 08:11 YourWishes

I don't use TypeScript so any help is appreciated. Feel free to open PRs to improve the type definitions.

lpinca avatar Dec 01 '19 06:12 lpinca

I also noticed quite a few type definitions are out of date. Specifically;

  • IOrder is missing properties like admin_graphql_api_id (and others)
  • IOrderFulfillment is missing properties like name
  • IOrderFulfillmentLineItem is missing the discount_allocations property.

I'm happy to submit a pull request when I get a chance, but I was wondering if this would impact users on a different API version? and how we would go about making that distinction...

ozzyonfire avatar Apr 20 '22 20:04 ozzyonfire