dwyl/aws-sdk-mock

Not possible to mock nested services using 5.6.0

thomaux opened this issue ยท 17 comments

Self reporting this, with the stricter typings TypeScript throws an error with the following:

awsSdkMock.mock('DynamoDB.DocumentClient', 'put', replaceFn);

This is because the ClientName type is defined as keyof typeof AWS which does not pick up the nested services. I'm already looking for a solution for this.

@thomaux thanks for proactively reporting this. ๐Ÿ‘
If you have time to fix, we will prioritise review/merge & publish. ๐Ÿ“ฆ

I have a patch over at thomaux@0237756. It's not great (I personally don't like the @ts-ignore comments in the definition file) but at least TypeScript recognizes nested services and their methods.

I could spend some more time on it in the hopes of doing away with those ignore comments, but if need be, this fix could be released as a patch version.

Please review.

@thomaux agree the workaround isn't great but the context makes it clear.
Please create a PR. and if you don't mind, please add comments explaining why the code is like that. ๐Ÿ‘

I think you might've forgotten to update the functions other than mockAWS.mock.

5.6.1 fixes 35/46 type issues we faced with 5.6.0, but there's still issues with mockAWS.remock and mockAWS.restore, possibly others.

You're absolutely right. My bad, launching a PR to fix the other functions

Thanks for the amazingly quick intervention!

aws-sdk-mock@5.6.2 published to NPM contains the update made by @thomaux ๐Ÿ“ฆ :shipit:

Thanks for the amazingly quick intervention!

Thank you for testing and reporting!

I'm still having problems with the latest version (5.6.2)
(the second and third errors might just be related to the first but not sure)

No overload matches this call.
      Overload 1 of 3, '(service: "ACM" | "APIGateway" | "ApplicationAutoScaling" | "AppStream" | "AutoScaling" | "Batch" | "Budgets" | "CloudDirectory" | "CloudFormation" | "CloudFront" | "CloudHSM" | "CloudSearch" | ... 287 more ... | "AmplifyUIBuilder", method: "config" | ... 6 more ... | "endpoint", replace: ReplaceFn<...>): void', gave the following error.
        Argument of type 'AwsService' is not assignable to parameter of type '"ACM" | "APIGateway" | "ApplicationAutoScaling" | "AppStream" | "AutoScaling" | "Batch" | "Budgets" | "CloudDirectory" | "CloudFormation" | "CloudFront" | "CloudHSM" | "CloudSearch" | ... 287 more ... | "AmplifyUIBuilder"'.
      Overload 2 of 3, '(service: "DynamoDB.DocumentClient", method: keyof DocumentClient, replace: any): void', gave the following error.
        Argument of type 'AwsService' is not assignable to parameter of type '"DynamoDB.DocumentClient"'.
      Overload 3 of 3, '(service: "ACM" | "APIGateway" | "ApplicationAutoScaling" | "AppStream" | "AutoScaling" | "Batch" | "Budgets" | "CloudDirectory" | "CloudFormation" | "CloudFront" | "CloudHSM" | "CloudSearch" | ... 287 more ... | "AmplifyUIBuilder", method: "config" | ... 6 more ... | "endpoint", replace: any): void', gave the following error.
        Argument of type 'AwsService' is not assignable to parameter of type '"ACM" | "APIGateway" | "ApplicationAutoScaling" | "AppStream" | "AutoScaling" | "Batch" | "Budgets" | "CloudDirectory" | "CloudFormation" | "CloudFront" | "CloudHSM" | "CloudSearch" | ... 287 more ... | "AmplifyUIBuilder"'.

    54     AwsMock.mock(service, method, (params, callback) => {


test/infra/AwsMocks.ts:55:35 - error TS2698: Spread types may only be created from object types.

55         requests.push({ params: { ...params } }); // clone the object in case the object is later modified
                                     ~~~~~~~~~
test/infra/AwsMocks.ts:56:9 - error TS2349: This expression is not callable.
  Type 'never' has no call signatures.

56         callback(getError?.(), getData?.());

@DavidRigglemanININ could you share the calling code that results in this error? What service and method are you trying to mock that results in this error?

I've got this helper method, which TS isn't happy with

export function mockAwsRequests<TParams, TError, TData>(
    service: AwsService, 
    method: string, 
    getError?: () => TError, 
    getData?: () => TData
): Array<AwsRequest<TParams>> {
    const requests = new Array<AwsRequest<TParams>>();
    AwsMock.mock(service, method, (params, callback) => {
        requests.push({ params: { ...params } }); // clone the object in case the object is later modified
        callback(getError?.(), getData?.());
    });
    return requests;
}

And here's another function that breaks with the latest version

function mockAws<TParams, TError, TData>(service: AwsService, method: string, error?: TError, data?: TData): AwsRequest<TParams> {
    const request: AwsRequest<TParams> = { params: undefined };
    AwsMock.mock(service, method, (params, callback) => {
        request.params = params;
        callback(error, data);
    });
    return request;
}

@DavidRigglemanININ the package exposes all the types used by the (re)mock and restores methods. You could reuse those to avoid type conflicts?

Except that breaks good architecture in terms of abstraction/encapsulation. Right now I could swap out the library (if needed down the road at any point) without having to change many references throughout my code base.

I don't know how you are consuming those wrappers in your project, but this is the wrapper I have:

import {
  ClientName,
  MethodName,
  mock,
  NestedClientFullName,
  NestedClientName,
  NestedMethodName,
} from 'aws-sdk-mock';

export function mockAWS<C extends ClientName, NC extends NestedClientName<C>>(service: NestedClientFullName<C, NC>, method: NestedMethodName<C, NC>): jest.Mock;
export function mockAWS<C extends ClientName>(service: C, method: MethodName<C>): jest.Mock {
  const jestMock = jest.fn();
  mock(service, method, jestMock);
  return jestMock;
}

I don't ever use those types outside of the wrapper, so in case I should want to replace the underlying mock library, I only need to update this wrapper.

Alternatively you could copy the type definitions source code and expose them yourself as I don't see an easy way to get the nested naming type, e.g. DynamoDB.DocumentClient to match the types you are defining?

I was able to get things working with about 3 times the level of complexity that I had before. While I appreciate the effort put into making this project possible and you are of course free to modify the library the best way you see fit, it would be nice if semver versioning was followed and breaking non-backwards compatible changes weren't included in minor releases.

I understand your frustration. Know that we think about semver and breaking changes, but didn't account for wrapping methods such as yours that could have conflicting typings with the new updated ones from the library.

For the "standard" use of the library, this change shouldn't break anything.