Add WarningFilter handling for YellowBox

Summary:
This diff backports warning filter handling to yellow box. I also backported the skipped warning handling so that ignored patterns do not log to the console, same as LogBox.

Changelog: [Internal]

Reviewed By: gaearon

Differential Revision: D18573288

fbshipit-source-id: 5bf8e86f754adc808313d7ed02f98daaf65de98c

Libraries/YellowBox/Data/YellowBoxRegistry.js

@@ -12,8 +12,8 @@
 
 const YellowBoxWarning = require('./YellowBoxWarning');
 
-import type {Category} from './YellowBoxCategory';
-
+import type {Category, Message} from './YellowBoxCategory';
+import type {Stack} from './YellowBoxSymbolication';
 export type Registry = Map<Category, $ReadOnlyArray<YellowBoxWarning>>;
 
 export type Observer = (registry: Registry) => void;
@@ -32,25 +32,13 @@ let disabled = false;
 let projection = new Map();
 let updateTimeout = null;
 
-function isWarningIgnored(warning: YellowBoxWarning): boolean {
-  for (const pattern of ignorePatterns) {
-    if (pattern instanceof RegExp && pattern.test(warning.message.content)) {
-      return true;
-    } else if (
-      typeof pattern === 'string' &&
-      warning.message.content.includes(pattern)
-    ) {
-      return true;
-    }
-  }
-  return false;
-}
-
 function handleUpdate(): void {
   projection = new Map();
   if (!disabled) {
     for (const [category, warnings] of registry) {
-      const filtered = warnings.filter(warning => !isWarningIgnored(warning));
+      const filtered = warnings.filter(
+        warning => !YellowBoxRegistry.isWarningIgnored(warning.message),
+      );
       if (filtered.length > 0) {
         projection.set(category, filtered);
       }
@@ -67,18 +55,28 @@ function handleUpdate(): void {
 }
 
 const YellowBoxRegistry = {
-  add({
-    args,
-  }: $ReadOnly<{|
-    args: $ReadOnlyArray<mixed>,
-  |}>): void {
-    if (typeof args[0] === 'string' && args[0].startsWith('(ADVICE)')) {
-      return;
+  isWarningIgnored(message: Message): boolean {
+    for (const pattern of ignorePatterns) {
+      if (pattern instanceof RegExp && pattern.test(message.content)) {
+        return true;
+      } else if (
+        typeof pattern === 'string' &&
+        message.content.includes(pattern)
+      ) {
+        return true;
       }
-    const {category, message, stack} = YellowBoxWarning.parse({
-      args,
-    });
-
+    }
+    return false;
+  },
+  add({
+    category,
+    message,
+    stack,
+  }: $ReadOnly<{|
+    category: Category,
+    message: Message,
+    stack: Stack,
+  |}>): void {
     let warnings = registry.get(category);
     if (warnings == null) {
       warnings = [];

Libraries/YellowBox/Data/__tests__/YellowBoxRegistry-test.js

@@ -11,6 +11,7 @@
 
 'use strict';
 
+const YellowBoxWarning = require('../YellowBoxWarning');
 const YellowBoxCategory = require('../YellowBoxCategory');
 const YellowBoxRegistry = require('../YellowBoxRegistry');
 
@@ -34,7 +35,7 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('adds and deletes warnings', () => {
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     const {category: categoryA} = YellowBoxCategory.parse(['A']);
 
     expect(registry().size).toBe(1);
@@ -46,9 +47,9 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('clears all warnings', () => {
-    YellowBoxRegistry.add({args: ['A']});
-    YellowBoxRegistry.add({args: ['B']});
-    YellowBoxRegistry.add({args: ['C']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['C']}));
 
     expect(registry().size).toBe(3);
 
@@ -57,9 +58,9 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('sorts warnings in chronological order', () => {
-    YellowBoxRegistry.add({args: ['A']});
-    YellowBoxRegistry.add({args: ['B']});
-    YellowBoxRegistry.add({args: ['C']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['C']}));
 
     const {category: categoryA} = YellowBoxCategory.parse(['A']);
     const {category: categoryB} = YellowBoxCategory.parse(['B']);
@@ -71,7 +72,7 @@ describe('YellowBoxRegistry', () => {
       categoryC,
     ]);
 
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
 
     // Expect `A` to be hoisted to the end of the registry.
     expect(Array.from(registry().keys())).toEqual([
@@ -82,9 +83,9 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('ignores warnings matching patterns', () => {
-    YellowBoxRegistry.add({args: ['A!']});
-    YellowBoxRegistry.add({args: ['B?']});
-    YellowBoxRegistry.add({args: ['C!']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A!']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B?']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['C!']}));
     expect(registry().size).toBe(3);
 
     YellowBoxRegistry.addIgnorePatterns(['!']);
@@ -95,9 +96,9 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('ignores warnings matching regexs or pattern', () => {
-    YellowBoxRegistry.add({args: ['There are 4 dogs']});
-    YellowBoxRegistry.add({args: ['There are 3 cats']});
-    YellowBoxRegistry.add({args: ['There are H cats']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['There are 4 dogs']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['There are 3 cats']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['There are H cats']}));
     expect(registry().size).toBe(3);
 
     YellowBoxRegistry.addIgnorePatterns(['dogs']);
@@ -111,9 +112,9 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('ignores all warnings when disabled', () => {
-    YellowBoxRegistry.add({args: ['A!']});
-    YellowBoxRegistry.add({args: ['B?']});
-    YellowBoxRegistry.add({args: ['C!']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A!']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B?']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['C!']}));
     expect(registry().size).toBe(3);
 
     YellowBoxRegistry.setDisabled(true);
@@ -124,57 +125,54 @@ describe('YellowBoxRegistry', () => {
   });
 
   it('groups warnings by simple categories', () => {
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B']}));
     expect(registry().size).toBe(2);
   });
 
   it('groups warnings by format string categories', () => {
-    YellowBoxRegistry.add({args: ['%s', 'A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['%s', 'A']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['%s', 'B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['%s', 'B']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     expect(registry().size).toBe(2);
 
-    YellowBoxRegistry.add({args: ['B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B']}));
     expect(registry().size).toBe(3);
   });
 
   it('groups warnings with consideration for arguments', () => {
-    YellowBoxRegistry.add({args: ['A', 'B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A', 'B']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A', 'B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A', 'B']}));
     expect(registry().size).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A', 'C']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A', 'C']}));
     expect(registry().size).toBe(2);
 
-    YellowBoxRegistry.add({args: ['%s', 'A', 'A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['%s', 'A', 'A']}));
     expect(registry().size).toBe(3);
 
-    YellowBoxRegistry.add({args: ['%s', 'B', 'A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['%s', 'B', 'A']}));
     expect(registry().size).toBe(3);
 
-    YellowBoxRegistry.add({args: ['%s', 'B', 'B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['%s', 'B', 'B']}));
     expect(registry().size).toBe(4);
   });
 
-  it('ignores warnings starting with "(ADVICE)"', () => {
-    YellowBoxRegistry.add({args: ['(ADVICE) ...']});
-    expect(registry().size).toBe(0);
-  });
-
   it('does not ignore warnings formatted to start with "(ADVICE)"', () => {
-    YellowBoxRegistry.add({args: ['%s ...', '(ADVICE)']});
+    YellowBoxRegistry.add(
+      YellowBoxWarning.parse({args: ['%s ...', '(ADVICE)']}),
+    );
     expect(registry().size).toBe(1);
   });
 
@@ -189,8 +187,8 @@ describe('YellowBoxRegistry', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A']});
-    YellowBoxRegistry.add({args: ['B']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['B']}));
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
   });
@@ -207,7 +205,7 @@ describe('YellowBoxRegistry', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
 
@@ -226,7 +224,7 @@ describe('YellowBoxRegistry', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    YellowBoxRegistry.add({args: ['A']});
+    YellowBoxRegistry.add(YellowBoxWarning.parse({args: ['A']}));
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
 

Libraries/YellowBox/YellowBox.js

@@ -13,6 +13,8 @@
 const React = require('react');
 
 import type {Registry, IgnorePattern} from './Data/YellowBoxRegistry';
+import YellowBoxWarning from './Data/YellowBoxWarning';
+
 import * as LogBoxData from '../LogBox/Data/LogBoxData';
 import NativeLogBox from '../NativeModules/specs/NativeLogBox';
 
@@ -78,15 +80,10 @@ if (__DEV__) {
         return;
       }
       errorImpl = function(...args) {
-        error.call(console, ...args);
-        // Show YellowBox for the `warning` module.
-        if (typeof args[0] === 'string' && args[0].startsWith('Warning: ')) {
-          registerWarning(...args);
-        }
+        registerError(...args);
       };
 
       warnImpl = function(...args) {
-        warn.call(console, ...args);
         registerWarning(...args);
       };
 
@@ -151,7 +148,53 @@ if (__DEV__) {
   };
 
   const registerWarning = (...args): void => {
-    YellowBoxRegistry.add({args});
+    if (typeof args[0] === 'string' && args[0].startsWith('(ADVICE)')) {
+      return;
+    }
+
+    const {category, message, stack} = YellowBoxWarning.parse({
+      args,
+    });
+
+    if (!YellowBoxRegistry.isWarningIgnored(message)) {
+      YellowBoxRegistry.add({category, message, stack});
+      warn.call(console, ...args);
+    }
+  };
+
+  const registerError = (...args): void => {
+    // Only show YellowBox for the `warning` module, otherwise pass through and skip.
+    if (typeof args[0] !== 'string' || !args[0].startsWith('Warning: ')) {
+      error.call(console, ...args);
+      return;
+    }
+
+    const format = args[0].replace('Warning: ', '');
+    const filterResult = LogBoxData.checkWarningFilter(format);
+    if (filterResult.suppressCompletely) {
+      return;
+    }
+
+    args[0] = filterResult.finalFormat;
+    const {category, message, stack} = YellowBoxWarning.parse({
+      args,
+    });
+
+    if (YellowBoxRegistry.isWarningIgnored(message)) {
+      return;
+    }
+
+    if (filterResult.forceDialogImmediately === true) {
+      // This will pop a redbox. Do not downgrade. These are real bugs with same severity as throws.
+      error.call(console, message.content);
+    } else {
+      // Unfortunately, we need to add the Warning: prefix back so we don't show a redbox later.
+      args[0] = `Warning: ${filterResult.finalFormat}`;
+
+      // Note: YellowBox has no concept of "soft errors" so we're showing YellowBox for those.
+      YellowBoxRegistry.add({category, message, stack});
+      error.call(console, ...args);
+    }
   };
 } else {
   YellowBox = class extends React.Component<Props, State> {

Libraries/YellowBox/__tests__/YellowBox-test.js

@@ -12,7 +12,6 @@
 'use strict';
 
 import * as React from 'react';
-const YellowBox = require('../YellowBox');
 const YellowBoxRegistry = require('../Data/YellowBoxRegistry');
 const LogBoxData = require('../../LogBox/Data/LogBoxData');
 const render = require('../../../jest/renderer');
@@ -22,17 +21,52 @@ jest.mock('../../LogBox/LogBoxNotificationContainer', () => ({
   default: 'LogBoxNotificationContainer',
 }));
 
+type Overrides = {|
+  forceDialogImmediately?: boolean,
+  suppressDialog_LEGACY?: boolean,
+  suppressCompletely?: boolean,
+|};
+
+const setFilter = (options?: Overrides) => {
+  LogBoxData.setWarningFilter(format => ({
+    finalFormat: format,
+    forceDialogImmediately: false,
+    suppressDialog_LEGACY: false,
+    suppressCompletely: false,
+    monitorEvent: null,
+    monitorListVersion: 0,
+    monitorSampleRate: 0,
+    ...options,
+  }));
+};
+
+const install = () => {
+  const YellowBox = require('../YellowBox');
+  YellowBox.install();
+};
+
+const uninstall = () => {
+  const YellowBox = require('../YellowBox');
+  YellowBox.uninstall();
+};
+
 describe('YellowBox', () => {
   const {error, warn} = console;
+  const mockError = jest.fn();
+  const mockWarn = jest.fn();
 
   beforeEach(() => {
     jest.resetModules();
-    (console: any).error = jest.fn();
-    (console: any).warn = jest.fn();
+
+    mockError.mockClear();
+    mockWarn.mockClear();
+
+    (console: any).error = mockError;
+    (console: any).warn = mockWarn;
   });
 
   afterEach(() => {
-    YellowBox.uninstall();
+    uninstall();
     (console: any).error = error;
     (console: any).warn = warn;
   });
@@ -40,7 +74,7 @@ describe('YellowBox', () => {
   it('can set `disableYellowBox` after installing', () => {
     expect((console: any).disableYellowBox).toBe(undefined);
 
-    YellowBox.install();
+    install();
 
     expect((console: any).disableYellowBox).toBe(false);
     expect(YellowBoxRegistry.isDisabled()).toBe(false);
@@ -55,7 +89,7 @@ describe('YellowBox', () => {
     expect((console: any).disableYellowBox).toBe(undefined);
 
     (console: any).disableYellowBox = true;
-    YellowBox.install();
+    install();
 
     expect((console: any).disableYellowBox).toBe(true);
     expect(YellowBoxRegistry.isDisabled()).toBe(true);
@@ -64,38 +98,140 @@ describe('YellowBox', () => {
   it('registers warnings', () => {
     jest.mock('../Data/YellowBoxRegistry');
 
-    YellowBox.install();
+    install();
 
     expect(YellowBoxRegistry.add).not.toBeCalled();
     (console: any).warn('...');
     expect(YellowBoxRegistry.add).toBeCalled();
+    expect(mockWarn).toBeCalledTimes(1);
+    expect(mockWarn).toBeCalledWith('...');
   });
 
-  it('registers errors beginning with "Warning: "', () => {
+  it('registers errors', () => {
     jest.mock('../Data/YellowBoxRegistry');
 
-    YellowBox.install();
+    install();
 
     (console: any).error('...');
     expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockError).toBeCalledTimes(1);
+    expect(mockError).toBeCalledWith('...');
+  });
+
+  it('skips ADVICE warnings', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    install();
+
+    (console: any).warn('(ADVICE) Ignore me');
+    expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockWarn).not.toBeCalled();
+  });
+
+  it('skips ignored warnings', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    install();
+
+    (YellowBoxRegistry: any).isWarningIgnored.mockReturnValue(true);
+    (console: any).warn('Ignore me');
+    expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockWarn).not.toBeCalled();
+  });
+
+  it('registers Warning module errors with default options to YellowBox', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    setFilter();
+    install();
 
     (console: any).error('Warning: ...');
     expect(YellowBoxRegistry.add).toBeCalled();
+    expect(mockError).toBeCalled();
+    expect(mockError).toBeCalledTimes(1);
+    expect(mockWarn).not.toBeCalled();
+  });
+
+  it('skips Warning module errors with forceDialogImmediately', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    setFilter({
+      suppressCompletely: true,
+    });
+    install();
+
+    (console: any).error('Warning: ...');
+    expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockError).not.toBeCalled();
+    expect(mockWarn).not.toBeCalled();
+  });
+
+  it('registers Warning errors with forceDialogImmediately as console.error (with interpolation)', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    setFilter({
+      forceDialogImmediately: true,
+    });
+
+    install();
+
+    (console: any).error('Warning: %s', 'Something');
+    expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockWarn).not.toBeCalled();
+    expect(mockError).toBeCalledTimes(1);
+
+    // We expect this to be the interpolated value because we don't do interpolation downstream.
+    // We also strip the "Warning" prefix, otherwise the redbox would be skipped downstream.
+    expect(mockError).toBeCalledWith('Something');
+  });
+
+  it('registers Warning errors with suppressDialog_LEGACY to YellowBox', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    setFilter({
+      suppressDialog_LEGACY: true,
+    });
+
+    install();
+
+    (console: any).error('Warning: Something');
+    expect(YellowBoxRegistry.add).toBeCalledTimes(1);
+    expect(mockWarn).not.toBeCalled();
+    expect(mockError).toBeCalledTimes(1);
+
+    // We cannot strip the "Warning" prefix or it would pop a redbox.
+    expect(mockError).toBeCalledWith('Warning: Something');
+  });
+
+  it('skips Warning errors sent to YellowBox but ignored by patterns', () => {
+    jest.mock('../Data/YellowBoxRegistry');
+
+    setFilter({
+      suppressDialog_LEGACY: true,
+    });
+
+    install();
+    (YellowBoxRegistry: any).isWarningIgnored.mockReturnValue(true);
+
+    (console: any).error('Warning: ...');
+    expect(YellowBoxRegistry.add).not.toBeCalled();
+    expect(mockWarn).not.toBeCalled();
+    expect(mockError).not.toBeCalled();
   });
 
   it('if LogBox is enabled, installs and uninstalls LogBox', () => {
     jest.mock('../../LogBox/Data/LogBoxData');
     jest.mock('../Data/YellowBoxRegistry');
-
+    const YellowBox = require('../YellowBox');
     YellowBox.__unstable_enableLogBox();
-    YellowBox.install();
+    install();
 
     (console: any).warn('Some warning');
     expect(YellowBoxRegistry.add).not.toBeCalled();
     expect(LogBoxData.addLog).toBeCalled();
-    expect(YellowBox.__unstable_isLogBoxEnabled()).toBe(true);
+    expect(require('../YellowBox').__unstable_isLogBoxEnabled()).toBe(true);
 
-    YellowBox.uninstall();
+    uninstall();
     (LogBoxData.addLog: any).mockClear();
 
     (console: any).warn('Some warning');
@@ -106,8 +242,8 @@ describe('YellowBox', () => {
 
   it('throws if LogBox is enabled after YellowBox is installed', () => {
     jest.mock('../Data/YellowBoxRegistry');
-
-    YellowBox.install();
+    const YellowBox = require('../YellowBox');
+    install();
 
     expect(() => YellowBox.__unstable_enableLogBox()).toThrow(
       'LogBox must be enabled before AppContainer is required so that it can properly wrap the console methods.\n\nPlease enable LogBox earlier in your app.\n\n',
@@ -115,12 +251,16 @@ describe('YellowBox', () => {
   });
 
   it('should render YellowBoxContainer by default', () => {
+    const YellowBox = require('../YellowBox');
+
     const output = render.shallowRender(<YellowBox />);
 
     expect(output).toMatchSnapshot();
   });
 
   it('should render LogBoxNotificationContainer when LogBox is enabled', () => {
+    const YellowBox = require('../YellowBox');
+
     YellowBox.__unstable_enableLogBox();
 
     const output = render.shallowRender(<YellowBox />);