maintain api compatibility with existing monday-sdk-js

[codyfrisch]

Right now it seems this project is implementing the variables as standalone second argument. This breaks API compatibility with the old monday-sdk-js implementation.

By adding an .api() method with the same arguments, as well as options to set the token or the API version then compatibility (for the most part) could be maintained.

There will also need to be an option on the constructor to tell graphql-request not to throw on error to maintain the behavior of the older fetch based client.

[RomKadria]

Hey, can you send an example for the error you are speaking about ?

[codyfrisch]

I'm not getting an error but the existing monday-sdk-js client takes the following to make API calls:

mondayClient.api(QUERY_STRING, options)

Options is an object that has keys for the token, the variables, and the apiVersion for runtime use, as shown in the code below. Before mondayApiClient.execute() or this._localApi is called, the .api() method repackages its arguments to match the interfaces of the actual clients.

` api(query, options = {}) { const params = { query, variables: options.variables }; const token = options.token || this._apiToken; const apiVersion = options.apiVersion || this._apiVersion;

let responsePromise;
if (token) {
  responsePromise = mondayApiClient.execute(params, token, { apiVersion });
} else {
  responsePromise = this._localApi("api", { params, apiVersion }).then(result => result.data);
}

return responsePromise.then(logWarnings);

}`

This new package on the other hand uses the same interface as mondayApiClient.execute() which isn't what any existing code was developed against (as its not exported)

mondayClient.query(QUERY_STRING, queryVarsObjectOrJSONString)

public query = async <T>(query: string, variables?: QueryVariables): Promise<T> => { const res = await this.client.request<T>(query, variables); return res; };

there should be a .api() method which takes an object with a variables key, like the old package. This way, developers can initialize this new client once at the start of their code and have it then function wherever they used the reference to that class instance with no additional code changes.

Imagine running monday-code, a request comes in, client gets initialized with the correct API token, now the client instance gets passed around as needed until the request is fulfilled. a .api() method with the same interface means the developer has no rewrite beyond instantiating the new class one place in their code - potentially covering all of their routes if they are creating it in a middleware for every request.

[codyfrisch]

As far as the "error" the current monday-sdk-js does not throw errors on anything since its based on fetch. graphql-request on the other hand will throw a JS exception for GraphQL errors by default. This is different than the monday-sdk-js behavior.

See https://github.com/jasonkuhrt/graphql-request/blob/main/README.md#errorpolicy

The existing monday-sdk-js uses a policy akin to the "all" policy of graphql-request. so any .api() method would use this.client.rawRequest rather than this.client.request.

My hope with all of this is that this package can dropped in for the other when instantiating the client in code, and that instance functions the same as the old client. That lets developers get this new package into use without breaking existing code. Then they can work through each query/mutation in their application and modify it to adopt the newer clients full features - and proper error handling when it arrives. Without having to use both clients during a transition period.

I'm presuming part of the goal of this project is to handle the compliant errors when they are released.

[RomKadria]

Hey, let me recap to make sure we are on the same page, and please correct me if we're not (beside the keyword of .query that we discuss in another issue) the sdk is good as it is right now, But its not backward compatible, so we should add another (that will be deprecated on release) method of .api that'l run rawRequest instead of request method, get its params the same way the old sdk does.

Am I correct here?