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

adds mocha polyfill to clear it.each errors #3383

Merged
merged 1 commit into from
May 13, 2021

Conversation

dimitropoulos
Copy link
Contributor

@dimitropoulos dimitropoulos commented May 13, 2021

Wherever it.each is called, but only in insomnia-inso and insomnia-app, we see the following error:

if you dig in, you'll see that this is due to these projects importing insomnia-testing which uses the Mocha types. What's shown here is that TypeScript is getting the types from two places for the same global definition for it (look at the pane on the right):

so now, going back to each, if you hover over the error it tells you where there's a problem:

Jest defines:

declare var it: jest.It;

while Mocha defines:

declare var it: Mocha.TestFunction;

So we know it's from Mocha because the error complained about TestFunction. Another way we know is you can just look at the jest.It definition, and sure enough, you'll see that each is defined.

The problem, then, is that mocha's TestFunction is missing each and since they're both merged global definitions, that isn't something TypeScript will allow. The thing is, though... we're not using Mocha. There's no way to tell TypeScript this since they're both globals.

Other people seemed to solve this by making it a dev dependency (cypress-io/cypress#6690 (comment), soundcloud/intervene#3) but it already is for us, and, anyway, our situation is different because we actually need to consume and re-export these types from insomnia-testing so this wouldn't solve our issue.

This isn't a problem for other packages in the project, e.g. o2k (see generate.test.ts) which do not import insomnia-testing.

Therefore, I've opted to solve this by adding a fake ambient definition for Mocha.TestFunction['each']. This makes the errors go away and I don't think there's any real risk to us because we're not actually using mocha.


That said, I'll mention @reynolek that the long-term fix for us (in my non-product-person non-decision-making opinion) is to remove mocha from all projects and just use jest and jest-runner for insomnia-testing. There are many other reasons to do this, but this hack we have to now impose is just another one to add to the pile.

@develohpanda
Copy link
Contributor

Gosh this is unfortunate, nice find.

Let's say at some future point in time we drop mocha and use jest-runner in insomnia-testing - could we bump into a similar conflict? I would hope not because I'd hope jest-runner is purely just that, a runner

@dimitropoulos
Copy link
Contributor Author

Nope, the conflict would be gone because the mocha types would be nowhere to be found in our project.

@dimitropoulos dimitropoulos merged commit 5a1f793 into Kong:develop May 13, 2021
@dimitropoulos dimitropoulos deleted the mocha-types-collision branch May 13, 2021 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants