Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 1.0.0 breaks Typescript/Mocha Workflow #101

Closed
1 task done
hzzzln opened this issue Jun 23, 2022 · 12 comments · Fixed by #112
Closed
1 task done

Version 1.0.0 breaks Typescript/Mocha Workflow #101

hzzzln opened this issue Jun 23, 2022 · 12 comments · Fixed by #112
Labels
bug Something isn't working waiting for reply

Comments

@hzzzln
Copy link

hzzzln commented Jun 23, 2022

Checklist

  • I have read Caveats documentation and didn't find a solution for this problem there.

Bug description

We use this library in our tests using Typescript and Mocha as a test runner. We recently wanted to update to version 1.0.0, however, the newly added Jest dependency breaks compatibility with Mocha.

Reproduction

Doing

yarn init
yarn add -D typescript aws-sdk-client-mock @types/jest @types/mocha ts-jest
echo "import {mockClient} from 'aws-sdk-client-mock';" >> index.ts
node_modules/typescript/bin/tsc index.ts

will result in a lot of errors of the form

node_modules/@types/mocha/index.d.ts:2642:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'beforeEach' must be of type 'Lifecycle', but here has type 'HookFunction'.

2642 declare var beforeEach: Mocha.HookFunction;
                 ~~~~~~~~~~

  node_modules/@types/jest/index.d.ts:34:13
    34 declare var beforeEach: jest.Lifecycle;
                   ~~~~~~~~~~
    'beforeEach' was also declared here.

Environment

  • Node version: v16.13.2
  • Testing lib and version: Mocha 10
  • Typescript version: 4.7.4
  • AWS SDK v3 Client mock version: 1.0.0
  • AWS JS SDK libs and versions: doesn't matter
@hzzzln hzzzln added the bug Something isn't working label Jun 23, 2022
@m-radzikowski
Copy link
Owner

Hey @hzzzln,
first of all - thank you for the great reproduction snippet!

As for the error - I think the Mocha and Jest generally can't coexist. The same error you get doing:

yarn init -y
yarn add -D typescript @types/jest @types/mocha
echo "" >> index.ts
node_modules/typescript/bin/tsc index.ts

The newly added Jest matchers are helpers if you use Jest for unit testing. Otherwise, they should not impact your workflow with other testing libraries. You don't have to install jest or @types/jest to use aws-sdk-client-mock with Mocha.

Matchers are for Jest only, with other assertion libraries you don't get them.

Please let me know if that clarifies things.

@rbargholz
Copy link

I also use TypeScript and mocha for testing and got similar issues. We don't have @types/jest or jest installed in our project, and got this particular set of errors.

... //Unrelated @sinonjs/fake-timers errors
node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:1:23 - error TS2688: Cannot find type definition file for 'jest'.

1 /// <reference types="jest" />
                        ~~~~

node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:117:15 - error TS2694: Namespace 'global.jest' has no exported member 'MatcherContext'.

117     ctx: jest.MatcherContext;
                  ~~~~~~~~~~~~~~

node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:129:10 - error TS2694: Namespace 'global.jest' has no exported member 'CustomMatcherResult'.

129 }): jest.CustomMatcherResult;
             ~~~~~~~~~~~~~~~~~~~

node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:131:60 - error TS2694: Namespace 'global.jest' has no exported member 'CustomMatcher'.

131     [P in keyof AwsSdkJestMockBaseMatchers<unknown>]: jest.CustomMatcher;
                                                               ~~~~~~~~~~~~~

node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:134:61 - error TS2694: Namespace 'global.jest' has no exported member 'CustomMatcher'.

134     [P in keyof AwsSdkJestMockAliasMatchers<unknown>]: jest.CustomMatcher;
                                                                ~~~~~~~~~~~~~


Found 5 errors in the same file, starting at: node_modules/aws-sdk-client-mock/dist/types/jestMatchers.d.ts:1

This is with this reproduction:

npm init -y
npm i --save-dev typescript aws-sdk-client-mock @types/mocha @aws-sdk/types @types/sinon
echo "import {mockClient} from 'aws-sdk-client-mock';" >> index.ts
node_modules/typescript/bin/tsc --lib es2018 index.ts

@hzzzln
Copy link
Author

hzzzln commented Jun 27, 2022

Thanks for your reply @m-radzikowski ! Unfortunately, I'm running into the same errors as @rbargholz . Installing jest in the first place was motivated by these compile errors.

@m-radzikowski
Copy link
Owner

@hzzzln, @rbargholz - please check out the 2.0.0-beta.1 release - it includes @types/jest as a dependency which should eliminate those errors without you having to install it yourself.

@rbargholz
Copy link

@m-radzikowski Thank you, I will take a look at that.

@hzzzln
Copy link
Author

hzzzln commented Jun 27, 2022

Thanks again for pushing a new version so quickly. Now Jest indeed gets installed as a dependency, but I'm just back to the original errors about subsequent variable declarations. Is there maybe a way to make the Jest feature in aws-sdk-client-mock entirely optional? I.E., I don't want Jest, because it conflicts with Mocha, but I need Jest because aws-sdk-client-mock depends on it.

@m-radzikowski
Copy link
Owner

Now Jest indeed gets installed as a dependency, but I'm just back to the original errors about subsequent variable declarations.

Right 🤦‍♂️

A workaround would be to not have the Jest types as a dependency in the lib (like in v2.0.0-beta.0) and to enable TS skipLibCheck option in your tsconfig.json to ignore the missing types in the library. This is, however, not optimal, and forces all non-Jest users to enable skipLibCheck option.

As for making this optional dependency - I don't think the Node ecosystem supports something like that.

The only solution that will prevent errors for all users would be to extract the helper Jest matchers as a separate dependency that you can install next to the aws-sdk-client-mock. And that's what I'm leaning towards at the moment... Although I will give myself a few days to think about it and it will take me a moment to reorganize the repo into multiple libraries.

@hzzzln
Copy link
Author

hzzzln commented Jun 28, 2022

Splitting it up into multiple packages was also my first idea, but I imagined it would be a radical step and quite a bit of work. I was wondering if there was some kind of conditional import:

try {
  import './jestMatchers';
} catch {
  console.debug("Couldn't import optional jestMatchers, maybe jest is not installed?")
}

But after some superficial googling this is either not possible or at least not cleanly possible.

Please contact me if you need anything else in regards to this issue, and thank you for your time so far.

@rbargholz
Copy link

rbargholz commented Jun 29, 2022

I'm not an npm expert, but is another option here something that can be done with peer dependencies?

@m-radzikowski
Copy link
Owner

I'm not an npm expert, but is another option here something that can be done with peer dependencies?

Peer dependencies won't help. From NPM v7 they are installed by default, so you will get the @types/jest anyway. With YARN or older NPM, if they are not installed, you will get missing types errors.

@m-radzikowski
Copy link
Owner

Just released: v2.0.0-beta.2

Jest matchers were removed from the main library. They are now available as a separate dependency (aws-sdk-client-mock-jest).

Please use this beta version, if no one will report further problems I will release v2.0.0 based on it.

@timvlaer
Copy link

Thanks for the fix. I can confirm that v2.0.0-beta.2 did fix this issue for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting for reply
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants