Endpoints and types refactor for evm #787
Conversation
…into endpoints-and-types-refactor-for-evm
|
Test coverage
|
….com/MoralisWeb3/Moralis-JS-SDK into endpoints-and-types-refactor-for-evm
…into endpoints-and-types-refactor-for-evm
There was a problem hiding this comment.
@sogunshola *Request & *Response types should have same prefix as OperationId (same uppercase lowercase letters). You may run the yarn gen:client command for the @moralisweb3/client-evm-api and the @moralisweb3/client-sol-api packages and check that, the generated clients have correct references.
|
|
||
| // Exports | ||
|
|
||
| export type GetContractNfTsRequest = Camelize<Omit<RequestParams, 'chain' | 'address'>> & { |
There was a problem hiding this comment.
GetContract >> NfTs << Request -> NFTs
|
|
||
| // Exports | ||
|
|
||
| export type GetNftContractMetadataRequest = Camelize<Omit<RequestParams, 'chain' | 'address'>> & { |
There was a problem hiding this comment.
GetNftContractMetadataRequest -> GetNFTContractMetadataRequest
| function getRequestUrlParams(request: GetPairReservesRequest, core: Core) { | ||
| return { | ||
| chain: EvmChainResolver.resolve(request.chain, core).apiHex, | ||
| pairAddress: EvmAddress.create(request.pairAddress, core).lowercase, |
There was a problem hiding this comment.
Shouldn't it be pair_address as we have in openapi?
There was a problem hiding this comment.
I can see some renaming to camelcase across getRequestUrlParams functions. But as I understand params inside should be equal to query+path?
There was a problem hiding this comment.
Good point! The getRequestUrlParams should return names using by the API (especially search parameters, url parameters depend on the urlPathPattern property).
There was a problem hiding this comment.
I guess we simply can declare in all getRequestUrlParams functions the returned type RequestParams:
function getRequestUrlParams(request: GetPairAddressRequest, core: Core) : RequestParams { }
Nope, ignore my message above. There may be needed some type overwritings. For example in openapi we have block with number type, but in SDK it's a string
There was a problem hiding this comment.
Maybe make a type Record<keyof RequestParams, string | undefined>, but I don't like this approach since it makes code more complicated. I guess it'd be better to put this check to the tests
There was a problem hiding this comment.
There is a small problem here.
public prepareUrl(baseUrl: string, request: Request) {
const urlParams = this.operation.getRequestUrlParams(request, this.core);
let urlPath = this.operation.urlPathPattern;
for (const paramName of this.operation.urlPathParamNames) {
const paramValue = urlParams[paramName as string];
if (!paramValue) {
throw new Error(`Param ${paramName as string} is required`);
}
urlPath = urlPath.replace(`{${paramName as string}}`, paramValue as string);
}
const url = `${baseUrl}${urlPath}`;
const urlSearchParams: Record<string, string | string[]> = {};
Object.keys(urlParams)
.filter((paramName) => !this.operation.urlPathParamNames.includes(paramName as keyof Request))
.forEach((paramName) => {
const paramValue = urlParams[paramName];
if (paramValue) {
urlSearchParams[paramName] = paramValue;
}
});
return { url, urlSearchParams };
}in this logic, the urlPathParamNames is (keyof Request)[] which has been camelized so the paramName searched for in the urlParams has to be camelCased as well hence the change to camel case. this is not the case for the search parameters because this logic does not make use of urlSearchParamNames. To change this to snake case, we have to change the expected type for urlPathParamNames.
@b4rtaz @Y0moo
…into endpoints-and-types-refactor-for-evm
| ignoreBodyCheckOperationNames.forEach((name) => { | ||
| operation.bodyParamNames?.splice(operation.bodyParamNames.indexOf(name as never), 1); | ||
| }); |
There was a problem hiding this comment.
I think this is too complicated. You can simplify this:
const ignoreBodyCheckOperationNames = ['getContractEvents', 'uploadFolder'];
// ...
if (!ignoreBodyCheckOperationNames.includes(operation.name)) {
expect(bodyParamNames?.sort().join(',')).toBe(openApiBodyParamNames?.sort().join(','));
}There was a problem hiding this comment.
We want skip operations, not parameter names.
name: 'Pull request'
about: A new pull request
New Pull Request
Checklist
Issue Description
Related issue: #
FILL_THIS_OUTSolution Description