feat: make node-notifier an optional dependency

jestjs · Sep 9, 2019 · 62fce79 · 62fce79
1 parent 736edd2
commit 62fce79
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,8 +29,9 @@
 - `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764))
 - `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686))
 - `[jest-mock]` Fix for mockReturnValue overriding mockImplementationOnce ([#8398](https://github.com/facebook/jest/pull/8398))
-- `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859))
+- `[jest-reporters]` Make node-notifer an optional dependency ([#8918](https://github.com/facebook/jest/pull/8918))
 - `[jest-snapshot]` Distinguish empty string from external snapshot not written ([#8880](https://github.com/facebook/jest/pull/8880))
+- `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859))
 - `[jest-snapshot]` [**BREAKING**] Distinguish empty string from internal snapshot not written ([#8898](https://github.com/facebook/jest/pull/8898))
 - `[jest-transform]` Properly cache transformed files across tests ([#8890](https://github.com/facebook/jest/pull/8890))
 

diff --git a/packages/jest-reporters/package.json b/packages/jest-reporters/package.json
@@ -23,7 +23,6 @@
     "jest-runtime": "^24.9.0",
     "jest-util": "^24.9.0",
     "jest-worker": "^24.6.0",
-    "node-notifier": "^5.4.3",
     "slash": "^3.0.0",
     "source-map": "^0.6.0",
     "string-length": "^3.1.0"
@@ -39,6 +38,9 @@
     "@types/node-notifier": "^5.4.0",
     "strip-ansi": "^5.0.0"
   },
+  "optionalDependencies": {
+    "node-notifier": "^5.4.3"
+  },
   "engines": {
     "node": ">= 8"
   },

diff --git a/packages/jest-reporters/src/__tests__/notify_reporter.test.ts b/packages/jest-reporters/src/__tests__/notify_reporter.test.ts
@@ -207,6 +207,48 @@ test('test failure-change with moduleName', () => {
   });
 });
 
+describe('node-notifier is an optional dependency', () => {
+  beforeEach(() => {
+    jest.resetModules();
+  });
+
+  const ctor = () => {
+    const config = makeGlobalConfig({
+      notify: true,
+      notifyMode: 'success',
+      rootDir: 'some-test',
+    });
+    return new NotifyReporter(config, () => {}, initialContext);
+  };
+
+  test('without node-notifier uses mock function that throws an error', () => {
+    jest.doMock('node-notifier', () => {
+      const error: any = new Error("Cannot find module 'node-notifier'");
+      error.code = 'MODULE_NOT_FOUND';
+      throw error;
+    });
+
+    expect(ctor).toThrow(
+      'notify reporter requires optional dependeny node-notifier but it was not found',
+    );
+  });
+
+  test('throws the error when require throws an unexpected error', () => {
+    const error = new Error('unexpected require error');
+    jest.doMock('node-notifier', () => {
+      throw error;
+    });
+    expect(ctor).toThrow(error);
+  });
+
+  test('uses node-notifier when it is available', () => {
+    const mockNodeNotifier = {notify: jest.fn()};
+    jest.doMock('node-notifier', () => mockNodeNotifier);
+    const result = ctor();
+    expect(result['_notifier']).toBe(mockNodeNotifier);
+  });
+});
+
 afterEach(() => {
   jest.clearAllMocks();
 });
diff --git a/packages/jest-reporters/src/notify_reporter.ts b/packages/jest-reporters/src/notify_reporter.ts
@@ -10,7 +10,6 @@ import * as util from 'util';
 import exit = require('exit');
 import {Config} from '@jest/types';
 import {AggregatedResult} from '@jest/test-result';
-import {notify} from 'node-notifier';
 import {Context, TestSchedulerContext} from './types';
 import BaseReporter from './base_reporter';
 
@@ -19,6 +18,7 @@ const isDarwin = process.platform === 'darwin';
 const icon = path.resolve(__dirname, '../assets/jest_logo.png');
 
 export default class NotifyReporter extends BaseReporter {
+  private _notifier = loadNotifier();
   private _startRun: (globalConfig: Config.GlobalConfig) => any;
   private _globalConfig: Config.GlobalConfig;
   private _context: TestSchedulerContext;
@@ -78,7 +78,7 @@ export default class NotifyReporter extends BaseReporter {
         result.numPassedTests,
       );
 
-      notify({icon, message, title});
+      this._notifier.notify({icon, message, title});
     } else if (
       testsHaveRun &&
       !success &&
@@ -106,9 +106,9 @@ export default class NotifyReporter extends BaseReporter {
       const quitAnswer = 'Exit tests';
 
       if (!watchMode) {
-        notify({icon, message, title});
+        this._notifier.notify({icon, message, title});
       } else {
-        notify(
+        this._notifier.notify(
           {
             actions: [restartAnswer, quitAnswer],
             closeLabel: 'Close',
@@ -137,3 +137,17 @@ export default class NotifyReporter extends BaseReporter {
     this._context.firstRun = false;
   }
 }
+
+function loadNotifier(): typeof import('node-notifier') {
+  try {
+    return require('node-notifier');
+  } catch (err) {
+    if (err.code !== 'MODULE_NOT_FOUND') {
+      throw err;
+    } else {
+      throw Error(
+        'notify reporter requires optional dependeny node-notifier but it was not found',
+      );
+    }
+  }
+}