Repository: electron/fiddle
Pull request #610 (2021-03-18 23:39:51)

fix: 'decompress is not a function' error
* Fix a 'decompress is not a function' error caused by an incorrect import of the 'decompress' module in content.ts.
* Update the tests that allowed this bug to sneak in.

> ~~Note, this PR uses 'require' to load the decompress module.~~ Using `const decompress = import 'decompress'` works in code but fails in tests with an error that says "decompress_1.default is not a function". After a bit of searching it appears adding`"esModuleInterop": true,` to tsconfig.json is the standard fix this error, but adding that line causes new breakage where tests use extract-zip, semver, and octokit.

Edit: I got curious this weekend and wanted to learn more about import + require + [esModuleInterop](https://www.typescriptlang.org/tsconfig#esModuleInterop), so I went ahead and resolved these extract-zip / semver / octokit issues.

(Decompress export is [here](https://github.com/kevva/decompress/blob/84a8c1046946add1a6ae01c54dbebf312e4ffc85/index.js#L131) and its TS bindings are [here](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/decompress/index.d.ts).)

Edit 2: The 0.003% coverage drop is because this is PR removes more code than it adds. There is no new uncovered code.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/38069737/badge)](https://coveralls.io/builds/38069737)

Coverage remained the same at 93.056% when pulling **386311bf0c4b9002b009eff097037319ab4d47cd on fix/decompress-is-not-a-function** into **255a81d7a7b827d9f290ef72f2924efc9d89ae86 on master**.


Patch:
diff --git a/src/renderer/binary.ts b/src/renderer/binary.ts
index 6d7b875d..2432e388 100644
--- a/src/renderer/binary.ts
+++ b/src/renderer/binary.ts
@@ -1,6 +1,6 @@
 import * as fsType from 'fs-extra';
 import * as path from 'path';
-import * as extractZipType from 'extract-zip';
+import extract from 'extract-zip';
 
 import { VersionState } from '../interfaces';
 import { fancyImport } from '../utils/import';
@@ -258,9 +258,6 @@ function getDownloadPath(version: string): string {
  * @returns {Promise<void>}
  */
 async function unzip(zipPath: string, dir: string): Promise<void> {
-  const extract: typeof extractZipType = (await fancyImport<any>('extract-zip'))
-    .default;
-
   process.noAsar = true;
 
   try {
diff --git a/src/renderer/content.ts b/src/renderer/content.ts
index 3c34c1b7..a9dac3bc 100644
--- a/src/renderer/content.ts
+++ b/src/renderer/content.ts
@@ -7,7 +7,7 @@ import { readFiddle } from '../utils/read-fiddle';
 import * as fsType from 'fs-extra';
 import * as path from 'path';
 import * as semver from 'semver';
-import * as decompress from 'decompress';
+import decompress from 'decompress';
 
 // parent directory of all the downloaded template fiddles
 const TEMPLATES_DIR = path.join(USER_DATA_PATH, 'Templates');
diff --git a/src/utils/octokit.ts b/src/utils/octokit.ts
index 22bc4a62..67859def 100644
--- a/src/utils/octokit.ts
+++ b/src/utils/octokit.ts
@@ -1,7 +1,6 @@
 import { Octokit } from '@octokit/rest';
 import { AppState } from '../renderer/state';
 
-let _Octokit: typeof Octokit;
 let _octo: Octokit;
 
 /**
@@ -12,8 +11,7 @@ let _octo: Octokit;
  * @returns {Promise<typeof GitHubType>}
  */
 export async function getOctokit(appState?: AppState): Promise<Octokit> {
-  _Octokit = _Octokit || ((await import('@octokit/rest')) as any).default;
-  _octo = _octo || new _Octokit();
+  _octo = _octo || new Octokit();
 
   // You can load Gists without being authenticated,
   // but we get better rate limits when authenticated
diff --git a/tsconfig.json b/tsconfig.json
index 1d5bd5d6..8c132529 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,5 +1,6 @@
 {
   "compilerOptions": {
+    "esModuleInterop": true,
     "outDir": "./dist",
     "allowJs": true,
     "skipLibCheck": true,


Test patch:
diff --git a/tests/mocks/worker.ts b/tests/mocks/worker.ts
index 9305a742..b14ca9f3 100644
--- a/tests/mocks/worker.ts
+++ b/tests/mocks/worker.ts
@@ -1,4 +1,4 @@
-import * as decomment from 'decomment';
+import decomment from 'decomment';
 
 export default class MockWorker {
   public onmessage: (msg: any) => any;
diff --git a/tests/renderer/binary-spec.ts b/tests/renderer/binary-spec.ts
index 7b666367..0528a369 100644
--- a/tests/renderer/binary-spec.ts
+++ b/tests/renderer/binary-spec.ts
@@ -9,12 +9,11 @@ import { removeTypeDefsForVersion } from '../../src/renderer/fetch-types';
 import { overridePlatform, resetPlatform } from '../utils';
 
 import * as path from 'path';
+import extract from 'extract-zip';
 
 jest.mock('fs-extra');
+jest.mock('extract-zip');
 jest.mock('../../src/renderer/ipc', () => ({}));
-jest.mock('extract-zip', () => {
-  return jest.fn((_a, _b) => Promise.resolve());
-});
 jest.mock('../../src/renderer/constants', () => ({
   USER_DATA_PATH: 'user/data/',
 }));
@@ -23,9 +22,6 @@ jest.mock('../../src/utils/import', () => ({
     if (p === 'fs-extra') {
       return require('fs-extra');
     }
-    if (p === 'extract-zip') {
-      return { default: require('extract-zip') };
-    }
   },
 }));
 jest.mock('../../src/renderer/fetch-types', () => ({
@@ -207,14 +203,15 @@ describe('binary', () => {
 
     it('handles an error in the zip file', async () => {
       const { download } = require('@electron/get');
+
       download.mockReturnValue('/fake/path');
+      mockState.versions['3.0.0'] = { state: 'unknown' };
 
-      const mockZip = require('extract-zip');
-      mockZip.mockImplementationOnce((_a: any, _b: any, c: any) =>
-        c(new Error('bwap-bwap')),
-      );
+      (extract as jest.Mock).mockRejectedValue(new Error('bwap-bwap'));
 
       await setupBinary(mockState, 'v3.0.0');
+
+      expect(extract as jest.Mock).toHaveBeenCalledTimes(1);
     });
   });
 });
diff --git a/tests/renderer/npm-spec.ts b/tests/renderer/npm-spec.ts
index 933fb394..ec7c4481 100644
--- a/tests/renderer/npm-spec.ts
+++ b/tests/renderer/npm-spec.ts
@@ -1,5 +1,5 @@
 import { mocked } from 'ts-jest/utils';
-import * as decomment from 'decomment';
+import decomment from 'decomment';
 
 import {
   findModules,
diff --git a/tests/renderer/versions-spec.ts b/tests/renderer/versions-spec.ts
index 01c30789..6a9acf18 100644
--- a/tests/renderer/versions-spec.ts
+++ b/tests/renderer/versions-spec.ts
@@ -1,6 +1,5 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import semver from 'semver';
 import { RunnableVersion, VersionSource } from '../../src/interfaces';
 import {
   addLocalVersion,
@@ -24,12 +23,6 @@ const mockVersions: Array<Partial<RunnableVersion>> = [
   { version: 'test-2', localPath: '/test/path/2' },
 ];
 
-jest.mock('semver', () => ({
-  default: {
-    gte: jest.fn(),
-  },
-}));
-
 describe('versions', () => {
   describe('getDefaultVersion()', () => {
     it('handles a stored version', () => {
@@ -181,12 +174,6 @@ describe('versions', () => {
       );
       mockFetchOnce(mockUnpkgResponse.toString());
 
-      // return whether or not version in JSON is >=0.24.0
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(false);
-
       const result = await fetchVersions();
       const expected = [
         { version: '10.0.0-nightly.20200303' },
diff --git a/tests/utils/fancy-import-spec.ts b/tests/utils/fancy-import-spec.ts
index 01e7c488..8eac0921 100644
--- a/tests/utils/fancy-import-spec.ts
+++ b/tests/utils/fancy-import-spec.ts
@@ -1,10 +1,24 @@
 import { fancyImport } from '../../src/utils/import';
 
 describe('fancyImport()', () => {
-  it('imports modules', async () => {
-    const fancy = await fancyImport('fs');
-    const required = require('fs');
+  it('imports es6 modules', async () => {
+    const name = 'fs';
+    const required = require(name);
+    const imported: any = await fancyImport(name);
 
-    expect(fancy).toEqual(required);
+    // https://www.typescriptlang.org/tsconfig#esModuleInterop
+    // can inject a 'default' for compatibility, so the imports may
+    // differ from require in that respect. Remove that from the equation before comparing.
+    const importedMinusDefault = { ...imported };
+    delete importedMinusDefault.default;
+
+    expect(required).toEqual(importedMinusDefault);
+  });
+
+  it('imports cjs single-function modules', async () => {
+    const name = 'decompress';
+    const required = require(name);
+    const imported: any = await fancyImport(name);
+    expect(required).toEqual(imported.default);
   });
 });
diff --git a/tests/utils/octokit-spec.ts b/tests/utils/octokit-spec.ts
index 687b76c0..df24ee23 100644
--- a/tests/utils/octokit-spec.ts
+++ b/tests/utils/octokit-spec.ts
@@ -1,30 +1,22 @@
 import { getOctokit } from '../../src/utils/octokit';
 
-let mockRequired = 0;
 let mockConstructed = 0;
 
 jest.mock('@octokit/rest', () => ({
-  get default() {
-    mockRequired++;
-
-    return class MockOctokit {
-      public authenticate = jest.fn();
-
-      constructor() {
-        mockConstructed++;
-      }
-    };
+  Octokit: class MockOctokit {
+    authenticate: jest.Mock;
+    constructor() {
+      ++mockConstructed;
+      this.authenticate = jest.fn();
+    }
   },
 }));
 
 describe('octokit', () => {
   describe('getOctokit()', () => {
-    it('requires the Octokit only once', async () => {
-      let octokit = await getOctokit();
-      octokit = await getOctokit();
-
-      expect(octokit).toBeTruthy();
-      expect(mockRequired).toBe(1);
+    it('constructs the Octokit only once', async () => {
+      await getOctokit();
+      await getOctokit();
       expect(mockConstructed).toBe(1);
     });