From fe82f402a971d44039ef91ea86944a087fd46e5f Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 11 Nov 2019 16:10:23 -0800 Subject: [PATCH] LogBox - Rework how fatals and syntax errors are handled Summary: This diff makes a few changes to how errors and warnings are handled in LogBox: - Soft errors continue to notify the user collapsed. - Fatal errors pop up full screen over the content, but are dismissible. - Syntax errors pop up full screen, and are **not** dismissible. - Removed the "Reload" button on fatals, and added back the dismiss/close. - Removed all buttons from syntax errors so users are encouraged to fix it and safe without reloading. To do this we needed to: - Move the selectedLogIndex state out of the component and up to the rest of the log state. - Change the way popping a log works, it's now done by setting selectedLogIndex at log time instead of deriving it at render time, that means `selectedLogIndex` is now the sole state value for deciding if the log inspector is open or not - Whenever the state is updated, find a syntax error if it's there, instead of doing this at render time Changelog: [Internal] Reviewed By: cpojer Differential Revision: D18421089 fbshipit-source-id: d2c4937f666f1302ed1a7b1b9c6679b0509136c5 --- Libraries/LogBox/Data/LogBoxData.js | 53 ++- .../LogBox/Data/__tests__/LogBoxData-test.js | 191 +++++++-- Libraries/LogBox/LogBox.js | 8 + Libraries/LogBox/UI/LogBoxContainer.js | 45 +-- Libraries/LogBox/UI/LogBoxInspector.js | 2 +- Libraries/LogBox/UI/LogBoxInspectorFooter.js | 66 +--- Libraries/LogBox/UI/LogBoxInspectorHeader.js | 12 + Libraries/LogBox/UI/LogBoxLogNotification.js | 1 - .../UI/__tests__/LogBoxContainer-test.js | 364 ++++++------------ .../__tests__/LogBoxInspectorFooter-test.js | 32 +- .../__tests__/LogBoxInspectorHeader-test.js | 13 + .../__tests__/LogBoxLogNotification-test.js | 1 - .../LogBoxContainer-test.js.snap | 321 +++------------ .../LogBoxInspector-test.js.snap | 3 +- .../LogBoxInspectorFooter-test.js.snap | 268 +++---------- .../LogBoxInspectorHeader-test.js.snap | 43 +++ 16 files changed, 547 insertions(+), 876 deletions(-) diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index eb631be94ff..734f4b605c1 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -27,7 +27,11 @@ export type LogData = $ReadOnly<{| |}>; export type Observer = ( - $ReadOnly<{|logs: LogBoxLogs, isDisabled: boolean|}>, + $ReadOnly<{| + logs: LogBoxLogs, + isDisabled: boolean, + selectedLogIndex: number, + |}>, ) => void; export type IgnorePattern = string | RegExp; @@ -41,10 +45,33 @@ const ignorePatterns: Set = new Set(); let logs: LogBoxLogs = new Set(); let updateTimeout = null; let _isDisabled = false; +let _selectedIndex = -1; const LOGBOX_ERROR_MESSAGE = 'An error was thrown when attempting to render log messages via LogBox.'; +function getNextState() { + const logArray = Array.from(logs); + let index = logArray.length - 1; + while (index >= 0) { + // The latest syntax error is selected and displayed before all other logs. + if (logArray[index].level === 'syntax') { + return { + logs, + isDisabled: _isDisabled, + selectedLogIndex: index, + }; + } + index -= 1; + } + + return { + logs, + isDisabled: _isDisabled, + selectedLogIndex: _selectedIndex, + }; +} + export function reportLogBoxError( error: ExtendedError, componentStack?: string, @@ -79,9 +106,7 @@ function handleUpdate(): void { if (updateTimeout == null) { updateTimeout = setImmediate(() => { updateTimeout = null; - observers.forEach(({observer}) => - observer({logs, isDisabled: _isDisabled}), - ); + observers.forEach(({observer}) => observer(getNextState())); }); } } @@ -172,10 +197,12 @@ export function addException(error: ExceptionData): void { ); // Start symbolicating now so it's warm when it renders. + logs.add(newLog); + if (level === 'fatal') { + _selectedIndex = logs.size - 1; symbolicateLogLazy(newLog); } - logs.add(newLog); } handleUpdate(); @@ -186,12 +213,18 @@ export function addException(error: ExceptionData): void { } export function clear(): void { - if (logs.size > 0) { - logs.clear(); + const newLogs = Array.from(logs).filter(log => log.level === 'syntax'); + if (newLogs.length !== logs.size) { + logs = new Set(newLogs); handleUpdate(); } } +export function setSelectedLog(index: number): void { + _selectedIndex = index; + handleUpdate(); +} + export function clearWarnings(): void { const newLogs = Array.from(logs).filter(log => log.level !== 'warn'); if (newLogs.length !== logs.size) { @@ -201,7 +234,9 @@ export function clearWarnings(): void { } export function clearErrors(): void { - const newLogs = Array.from(logs).filter(log => log.level !== 'error'); + const newLogs = Array.from(logs).filter( + log => log.level !== 'error' && log.level !== 'fatal', + ); if (newLogs.length !== logs.size) { logs = new Set(newLogs); handleUpdate(); @@ -276,7 +311,7 @@ export function observe(observer: Observer): Subscription { const subscription = {observer}; observers.add(subscription); - observer({logs, isDisabled: _isDisabled}); + observer(getNextState()); return { unsubscribe(): void { diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index 064bd18a08c..19a313eb249 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -37,6 +37,12 @@ const disabledState = () => { return observer.mock.calls[0][0].isDisabled; }; +const selectedLogIndex = () => { + const observer = jest.fn(); + LogBoxData.observe(observer).unsubscribe(); + return observer.mock.calls[0][0].selectedLogIndex; +}; + const observe = () => { const observer = jest.fn(); return { @@ -59,7 +65,7 @@ const addLogs = logs => { }); }; -const addExceptions = errors => { +const addSoftErrors = errors => { errors.forEach(error => { LogBoxData.addException( Object.assign( @@ -81,8 +87,30 @@ const addExceptions = errors => { }); }; +const addFatalErrors = errors => { + errors.forEach(error => { + LogBoxData.addException( + Object.assign( + {}, + { + message: '', + originalMessage: '', + name: 'console.error', + componentStack: '', + stack: [], + id: 0, + isFatal: true, + }, + typeof error === 'string' + ? {message: error, originalMessage: error} + : error, + ), + ); + }); +}; + const addSyntaxError = () => { - addExceptions([ + addFatalErrors([ { message: ` @@ -109,13 +137,19 @@ beforeEach(() => { describe('LogBoxData', () => { it('adds and dismisses logs', () => { addLogs(['A']); - addExceptions(['B']); + addSoftErrors(['B']); + addFatalErrors(['C']); + addSyntaxError(); jest.runAllImmediates(); - expect(registry().length).toBe(2); + expect(registry().length).toBe(4); expect(registry()[0]).toBeDefined(); expect(registry()[1]).toBeDefined(); + LogBoxData.dismiss(registry()[0]); + expect(registry().length).toBe(3); + LogBoxData.dismiss(registry()[0]); + expect(registry().length).toBe(2); LogBoxData.dismiss(registry()[0]); expect(registry().length).toBe(1); LogBoxData.dismiss(registry()[0]); @@ -123,20 +157,22 @@ describe('LogBoxData', () => { expect(registry()[0]).toBeUndefined(); }); - it('clears all logs', () => { + it('clears all logs except syntax errors', () => { addLogs(['A', 'B']); - addExceptions(['C', 'D']); + addSoftErrors(['C', 'D']); + addFatalErrors(['E', 'F']); + addSyntaxError(); jest.runAllImmediates(); - expect(registry().length).toBe(4); + expect(registry().length).toBe(7); LogBoxData.clear(); - expect(registry().length).toBe(0); + expect(registry().length).toBe(1); }); it('clears only warnings', () => { addLogs(['A', 'B']); - addExceptions(['C', 'D', 'E']); + addSoftErrors(['C', 'D', 'E']); addSyntaxError(); jest.runAllImmediates(); @@ -146,13 +182,14 @@ describe('LogBoxData', () => { expect(registry().length).toBe(4); }); - it('clears only errors', () => { + it('clears errors and fatals, but not syntax errors.', () => { addLogs(['A', 'B']); - addExceptions(['C', 'D', 'E']); + addSoftErrors(['C', 'D', 'E']); + addFatalErrors(['F']); addSyntaxError(); jest.runAllImmediates(); - expect(registry().length).toBe(6); + expect(registry().length).toBe(7); LogBoxData.clearErrors(); expect(registry().length).toBe(3); @@ -160,23 +197,25 @@ describe('LogBoxData', () => { it('clears only syntax errors', () => { addLogs(['A', 'B']); - addExceptions(['C', 'D', 'E']); + addSoftErrors(['C', 'D', 'E']); + addFatalErrors(['F']); addSyntaxError(); jest.runAllImmediates(); - expect(registry().length).toBe(6); + expect(registry().length).toBe(7); LogBoxData.clearSyntaxErrors(); - expect(registry().length).toBe(5); + expect(registry().length).toBe(6); }); it('clears all types', () => { addLogs(['A', 'B']); - addExceptions(['C', 'D', 'E']); + addSoftErrors(['C', 'D', 'E']); + addFatalErrors(['F']); addSyntaxError(); jest.runAllImmediates(); - expect(registry().length).toBe(6); + expect(registry().length).toBe(7); LogBoxData.clearErrors(); LogBoxData.clearWarnings(); @@ -186,26 +225,103 @@ describe('LogBoxData', () => { it('keeps logs in chronological order', () => { addLogs(['A']); - addExceptions(['B']); - addLogs(['C']); + addSoftErrors(['B']); + addFatalErrors(['C']); + addLogs(['D']); jest.runAllImmediates(); let logs = registry(); - expect(logs.length).toBe(3); + expect(logs.length).toBe(4); expect(logs[0].category).toEqual('A'); expect(logs[1].category).toEqual('B'); expect(logs[2].category).toEqual('C'); + expect(logs[3].category).toEqual('D'); addLogs(['A']); jest.runAllImmediates(); // Expect `A` to be added to the end of the registry. logs = registry(); - expect(logs.length).toBe(4); + expect(logs.length).toBe(5); expect(logs[0].category).toEqual('A'); expect(logs[1].category).toEqual('B'); expect(logs[2].category).toEqual('C'); - expect(logs[3].category).toEqual('A'); + expect(logs[3].category).toEqual('D'); + expect(logs[4].category).toEqual('A'); + }); + + it('sets the selectedLogIndex', () => { + expect(selectedLogIndex()).toBe(-1); + + LogBoxData.setSelectedLog(1); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(1); + }); + + it('does not set the selectedLogIndex for warnings', () => { + addLogs(['A']); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(-1); + }); + + it('does not set the selectedLogIndex for soft errors', () => { + addSoftErrors(['A']); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(-1); + }); + + it('sets the selectedLogIndex to the last fatal error', () => { + addFatalErrors(['A']); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); + + addLogs(['B']); + addFatalErrors(['C']); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(2); + }); + + it('sets the selectedLogIndex to the last syntax error', () => { + addSyntaxError(); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); + + addLogs(['B']); + addSyntaxError(); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(2); + }); + + it('keeps selectedLogIndex set to the syntax error even when a new fatal is added', () => { + addSyntaxError(); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); + + addLogs(['B']); + addFatalErrors(['C']); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); + }); + + it('keeps selectedLogIndex set to the syntax error even when explicitly changed', () => { + addSyntaxError(); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); + + LogBoxData.setSelectedLog(1); + jest.runAllImmediates(); + + expect(selectedLogIndex()).toBe(0); }); it('increments the count of previous log with matching category (logs)', () => { @@ -231,8 +347,8 @@ describe('LogBoxData', () => { expect(logs[1].count).toBe(2); }); - it('increments the count of previous log with matching category (exceptions)', () => { - addExceptions(['A', 'B']); + it('increments the count of previous log with matching category (errors)', () => { + addFatalErrors(['A', 'B']); jest.runAllImmediates(); let logs = registry(); @@ -242,7 +358,7 @@ describe('LogBoxData', () => { expect(logs[1].category).toEqual('B'); expect(logs[1].count).toBe(1); - addExceptions(['B']); + addSoftErrors(['B']); jest.runAllImmediates(); // Expect `B` to be rolled into the last log. @@ -283,8 +399,9 @@ describe('LogBoxData', () => { expect(filteredRegistry().length).toBe(0); }); - it('ignores logs matching patterns (exceptions)', () => { - addExceptions(['A!', 'B?', 'C!']); + it('ignores logs matching patterns (errors)', () => { + addSoftErrors(['A!', 'B?']); + addFatalErrors(['C!']); jest.runAllImmediates(); expect(filteredRegistry().length).toBe(3); @@ -312,8 +429,9 @@ describe('LogBoxData', () => { expect(filteredRegistry().length).toBe(0); }); - it('ignores matching regexs or pattern (exceptions)', () => { - addExceptions(['There are 4 dogs', 'There are 3 cats', 'There are H cats']); + it('ignores matching regexs or pattern (errors)', () => { + addSoftErrors(['There are 4 dogs', 'There are 3 cats']); + addFatalErrors(['There are H cats']); jest.runAllImmediates(); expect(filteredRegistry().length).toBe(3); @@ -329,8 +447,9 @@ describe('LogBoxData', () => { }); it('ignores all logs except fatals when disabled', () => { - addLogs(['A!', 'B?']); - addExceptions(['C!']); + addLogs(['A!']); + addSoftErrors(['B?']); + addFatalErrors(['C!']); addSyntaxError(); jest.runAllImmediates(); @@ -424,7 +543,9 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); - addExceptions(['B']); + addSoftErrors(['B']); + addFatalErrors(['C']); + addSyntaxError(); jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); @@ -443,7 +564,9 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); - addExceptions(['B']); + addSoftErrors(['B']); + addFatalErrors(['C']); + addSyntaxError(); jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); @@ -462,6 +585,8 @@ describe('LogBoxData', () => { expect(observer.mock.calls.length).toBe(1); addLogs(['A']); + addSoftErrors(['B']); + addFatalErrors(['C']); addSyntaxError(); jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); diff --git a/Libraries/LogBox/LogBox.js b/Libraries/LogBox/LogBox.js index 3f531253761..3c08035feee 100644 --- a/Libraries/LogBox/LogBox.js +++ b/Libraries/LogBox/LogBox.js @@ -26,6 +26,7 @@ type State = {| logs: LogBoxLogs, isDisabled: boolean, hasError: boolean, + selectedLogIndex: number, |}; let LogBoxComponent; @@ -109,6 +110,7 @@ if (__DEV__) { logs: new Set(), isDisabled: false, hasError: false, + selectedLogIndex: -1, }; render(): React.Node { @@ -125,6 +127,8 @@ if (__DEV__) { onDismissErrors={LogBoxData.clearErrors} logs={this.state.logs} isDisabled={this.state.isDisabled} + selectedLogIndex={this.state.selectedLogIndex} + setSelectedLog={this._handleSetSelectedLog} /> ); } @@ -144,6 +148,10 @@ if (__DEV__) { _handleDismiss(log: LogBoxLog): void { LogBoxData.dismiss(log); } + + _handleSetSelectedLog(index: number): void { + LogBoxData.setSelectedLog(index); + } }; const registerWarning = (...args): void => { diff --git a/Libraries/LogBox/UI/LogBoxContainer.js b/Libraries/LogBox/UI/LogBoxContainer.js index 1dddb9ca426..ccfd1730724 100644 --- a/Libraries/LogBox/UI/LogBoxContainer.js +++ b/Libraries/LogBox/UI/LogBoxContainer.js @@ -23,12 +23,14 @@ type Props = $ReadOnly<{| onDismiss: (log: LogBoxLog) => void, onDismissWarns: () => void, onDismissErrors: () => void, + setSelectedLog: number => void, logs: LogBoxLogs, + selectedLogIndex: number, isDisabled?: ?boolean, |}>; function LogBoxContainer(props: Props): React.Node { - const [selectedLogIndex, setSelectedLog] = React.useState(null); + const {selectedLogIndex, setSelectedLog} = props; const logs = Array.from(props.logs); @@ -38,7 +40,7 @@ function LogBoxContainer(props: Props): React.Node { // is now outside the bounds of the log array. if (selectedLogIndex != null) { if (logs.length - 1 <= 0) { - setSelectedLog(null); + setSelectedLog(-1); } else if (selectedLogIndex >= logs.length - 1) { setSelectedLog(selectedLogIndex - 1); } @@ -47,7 +49,7 @@ function LogBoxContainer(props: Props): React.Node { } function handleInspectorMinimize() { - setSelectedLog(null); + setSelectedLog(-1); } function openLog(log: LogBoxLog) { @@ -60,16 +62,7 @@ function LogBoxContainer(props: Props): React.Node { setSelectedLog(index); } - let fatalIndex = logs.length - 1; - while (fatalIndex >= 0) { - const level = logs[fatalIndex].level; - if (level === 'fatal' || level === 'syntax') { - break; - } - fatalIndex -= 1; - } - - if (selectedLogIndex == null && fatalIndex >= 0) { + if (selectedLogIndex > -1) { return ( - - ); - } - - if (selectedLogIndex != null) { - return ( - - = 0 ? logs[fatalIndex].level : null} selectedIndex={selectedLogIndex} /> @@ -104,7 +81,9 @@ function LogBoxContainer(props: Props): React.Node { } const warnings = logs.filter(log => log.level === 'warn'); - const errors = logs.filter(log => log.level === 'error'); + const errors = logs.filter( + log => log.level === 'error' || log.level === 'fatal', + ); return ( {warnings.length > 0 && ( @@ -114,9 +93,6 @@ function LogBoxContainer(props: Props): React.Node { level="warn" totalLogCount={warnings.length} onPressOpen={() => openLog(warnings[warnings.length - 1])} - onPressList={() => { - /* TODO: open log list */ - }} onPressDismiss={props.onDismissWarns} /> @@ -128,9 +104,6 @@ function LogBoxContainer(props: Props): React.Node { level="error" totalLogCount={errors.length} onPressOpen={() => openLog(errors[errors.length - 1])} - onPressList={() => { - /* TODO: open log list */ - }} onPressDismiss={props.onDismissErrors} /> diff --git a/Libraries/LogBox/UI/LogBoxInspector.js b/Libraries/LogBox/UI/LogBoxInspector.js index bc86f01e13f..111ac93ebc6 100644 --- a/Libraries/LogBox/UI/LogBoxInspector.js +++ b/Libraries/LogBox/UI/LogBoxInspector.js @@ -79,7 +79,7 @@ function LogBoxInspector(props: Props): React.Node { ); diff --git a/Libraries/LogBox/UI/LogBoxInspectorFooter.js b/Libraries/LogBox/UI/LogBoxInspectorFooter.js index 7569cd861e3..62d81d80f0a 100644 --- a/Libraries/LogBox/UI/LogBoxInspectorFooter.js +++ b/Libraries/LogBox/UI/LogBoxInspectorFooter.js @@ -23,31 +23,16 @@ import * as LogBoxStyle from './LogBoxStyle'; type Props = $ReadOnly<{| onDismiss: () => void, onMinimize: () => void, - fatalType?: ?LogLevel, + level?: ?LogLevel, |}>; function LogBoxInspectorFooter(props: Props): React.Node { - if (props.fatalType === 'fatal' || props.fatalType === 'syntax') { + if (props.level === 'syntax') { return ( - - - require('../../Utilities/DevSettings').reload('LogBox fatal') - } - style={fatalStyles.button}> - - Reload - - {{fatal: 'Fatal', syntax: 'Syntax'}[props.fatalType]} errors - require a full reload - - - - + + + This error cannot be dismissed. + ); } @@ -82,36 +67,6 @@ function FooterButton(props: ButtonProps): React.Node { ); } -const fatalStyles = StyleSheet.create({ - root: { - shadowColor: LogBoxStyle.getBackgroundDarkColor(1), - }, - button: { - flex: 1, - }, - content: { - alignItems: 'center', - height: 60, - justifyContent: 'center', - }, - subtext: { - height: 60, - }, - label: { - color: LogBoxStyle.getTextColor(1), - fontSize: 14, - fontWeight: '600', - includeFontPadding: false, - lineHeight: 20, - }, - subtextLabel: { - color: LogBoxStyle.getTextColor(0.8), - fontSize: 11, - includeFontPadding: false, - lineHeight: 12, - }, -}); - const buttonStyles = StyleSheet.create({ button: { flex: 1, @@ -139,6 +94,15 @@ const styles = StyleSheet.create({ elevation: 1, flexDirection: 'row', }, + syntaxErrorText: { + textAlign: 'center', + width: '100%', + fontSize: 14, + paddingTop: 15, + paddingBottom: 15, + fontStyle: 'italic', + color: LogBoxStyle.getTextColor(0.6), + }, }); export default LogBoxInspectorFooter; diff --git a/Libraries/LogBox/UI/LogBoxInspectorHeader.js b/Libraries/LogBox/UI/LogBoxInspectorHeader.js index 30efbc46852..b9272a883d1 100644 --- a/Libraries/LogBox/UI/LogBoxInspectorHeader.js +++ b/Libraries/LogBox/UI/LogBoxInspectorHeader.js @@ -29,6 +29,18 @@ type Props = $ReadOnly<{| |}>; function LogBoxInspectorHeader(props: Props): React.Node { + if (props.level === 'syntax') { + return ( + + + + Failed to compile + + + + ); + } + const prevIndex = props.selectedIndex - 1 < 0 ? props.total - 1 : props.selectedIndex - 1; const nextIndex = diff --git a/Libraries/LogBox/UI/LogBoxLogNotification.js b/Libraries/LogBox/UI/LogBoxLogNotification.js index 41cad233c92..8fc57f61905 100644 --- a/Libraries/LogBox/UI/LogBoxLogNotification.js +++ b/Libraries/LogBox/UI/LogBoxLogNotification.js @@ -27,7 +27,6 @@ type Props = $ReadOnly<{| totalLogCount: number, level: 'warn' | 'error', onPressOpen: () => void, - onPressList: () => void, onPressDismiss: () => void, |}>; diff --git a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js index 7eeead80044..c26713a4d77 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js @@ -23,6 +23,8 @@ describe('LogBoxContainer', () => { onDismiss={() => {}} onDismissWarns={() => {}} onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={-1} logs={new Set()} />, ); @@ -30,121 +32,119 @@ describe('LogBoxContainer', () => { expect(output).toMatchSnapshot(); }); - it('should render the latest warning', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'warn', - { - content: 'Some kind of message', - substitutions: [], - }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'warn', - { - content: 'Some kind of message (latest)', - substitutions: [], - }, - [], - 'Some kind of message (latest)', - [], - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render the latest error', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'error', - { - content: 'Some kind of message', - substitutions: [], - }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'error', - { - content: 'Some kind of message (latest)', - substitutions: [], - }, - [], - 'Some kind of message (latest)', - [], - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render both an error and warning', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'warn', - { - content: 'Some kind of message', - substitutions: [], - }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'error', - { - content: 'Some kind of message (latest)', - substitutions: [], - }, - [], - 'Some kind of message (latest)', - [], - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render null when disabled with non-fatal error and warning', () => { + it('should render null with no selected log and disabled', () => { const output = render.shallowRender( {}} onDismissWarns={() => {}} onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={-1} + logs={ + new Set([ + new LogBoxLog( + 'warn', + { + content: 'Some kind of message', + substitutions: [], + }, + [], + 'Some kind of message', + [], + ), + ]) + } + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render the latest warning notification', () => { + const output = render.shallowRender( + {}} + onDismissWarns={() => {}} + onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={-1} + logs={ + new Set([ + new LogBoxLog( + 'warn', + { + content: 'Some kind of message', + substitutions: [], + }, + [], + 'Some kind of message', + [], + ), + new LogBoxLog( + 'warn', + { + content: 'Some kind of message (latest)', + substitutions: [], + }, + [], + 'Some kind of message (latest)', + [], + ), + ]) + } + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render the latest error notification', () => { + const output = render.shallowRender( + {}} + onDismissWarns={() => {}} + onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={-1} + logs={ + new Set([ + new LogBoxLog( + 'error', + { + content: 'Some kind of message', + substitutions: [], + }, + [], + 'Some kind of message', + [], + ), + new LogBoxLog( + 'error', + { + content: 'Some kind of message (latest)', + substitutions: [], + }, + [], + 'Some kind of message (latest)', + [], + ), + ]) + } + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render both an error and warning notification', () => { + const output = render.shallowRender( + {}} + onDismissWarns={() => {}} + onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={-1} logs={ new Set([ new LogBoxLog( @@ -175,95 +175,15 @@ describe('LogBoxContainer', () => { expect(output).toMatchSnapshot(); }); - it('should render fatal before warn and error', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'fatal', - { - content: 'Some kind of fatal message', - substitutions: [], - }, - [], - 'Some kind of fatal message', - [], - ), - new LogBoxLog( - 'warn', - { - content: 'Some kind of message', - substitutions: [], - }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'error', - { - content: 'Some kind of message (latest)', - substitutions: [], - }, - [], - 'Some kind of message (latest)', - [], - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render most recent fatal', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'fatal', - { - content: 'Should not be selected', - substitutions: [], - }, - [], - 'Some kind of fatal message', - [], - ), - new LogBoxLog( - 'fatal', - { - content: 'Should be selected', - substitutions: [], - }, - [], - 'Some kind of message', - [], - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render fatal error even when disabled', () => { + it('should render selected fatal error even when disabled', () => { const output = render.shallowRender( {}} onDismissWarns={() => {}} onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={0} logs={ new Set([ new LogBoxLog( @@ -284,69 +204,15 @@ describe('LogBoxContainer', () => { expect(output).toMatchSnapshot(); }); - it('should render most recent syntax error', () => { - const output = render.shallowRender( - {}} - onDismissWarns={() => {}} - onDismissErrors={() => {}} - logs={ - new Set([ - new LogBoxLog( - 'syntax', - { - content: 'Should not be selected', - substitutions: [], - }, - [], - 'Some kind of syntax error message', - [], - { - fileName: - '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', - location: {row: 199, column: 0}, - content: ` 197 | }); - 198 | -> 199 | export default CrashReactApp; - | ^ - 200 |`, - }, - ), - new LogBoxLog( - 'syntax', - { - content: 'Should be selected', - substitutions: [], - }, - [], - 'Some kind of syntax error message', - [], - { - fileName: - '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', - location: {row: 199, column: 0}, - content: ` 197 | }); - 198 | -> 199 | export default CrashReactApp; - | ^ - 200 |`, - }, - ), - ]) - } - />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render syntax error even when disabled', () => { + it('should render selected syntax error even when disabled', () => { const output = render.shallowRender( {}} onDismissWarns={() => {}} onDismissErrors={() => {}} + setSelectedLog={() => {}} + selectedLogIndex={0} logs={ new Set([ new LogBoxLog( diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js index c498646ff6d..28a920600db 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js @@ -16,44 +16,48 @@ const LogBoxInspectorFooter = require('../LogBoxInspectorFooter').default; const render = require('../../../../jest/renderer'); describe('LogBoxInspectorFooter', () => { - it('should render two buttons for warning with no fatal', () => { - const output = render.shallowRender( - {}} onDismiss={() => {}} />, - ); - - expect(output).toMatchSnapshot(); - }); - - it('should render fatal for warning with a fatal', () => { + it('should render two buttons for warning', () => { const output = render.shallowRender( {}} onDismiss={() => {}} - fatalType="syntax" + level="warn" />, ); expect(output).toMatchSnapshot(); }); - it('should render fatal button', () => { + it('should render two buttons for error', () => { const output = render.shallowRender( {}} onDismiss={() => {}} - fatalType="fatal" + level="error" />, ); expect(output).toMatchSnapshot(); }); - it('should render fatal button for syntax erorr', () => { + it('should render two buttons for fatal', () => { const output = render.shallowRender( {}} onDismiss={() => {}} - fatalType="syntax" + level="fatal" + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render no buttons and a message for syntax error', () => { + const output = render.shallowRender( + {}} + onDismiss={() => {}} + level="syntax" />, ); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorHeader-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorHeader-test.js index b919f08e48b..4d1e11a35ab 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorHeader-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorHeader-test.js @@ -54,4 +54,17 @@ describe('LogBoxInspectorHeader', () => { expect(output).toMatchSnapshot(); }); + + it('should render syntax error header', () => { + const output = render.shallowRender( + {}} + selectedIndex={0} + total={1} + level="syntax" + />, + ); + + expect(output).toMatchSnapshot(); + }); }); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js b/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js index 8a40f529bd6..8adee4f11c1 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js @@ -35,7 +35,6 @@ describe('LogBoxLogNotification', () => { totalLogCount={1} level="warn" onPressOpen={() => {}} - onPressList={() => {}} onPressDismiss={() => {}} />, ); diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap index d47d1a24a0d..76aa260360f 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`LogBoxContainer should render both an error and warning 1`] = ` +exports[`LogBoxContainer should render both an error and warning notification 1`] = ` @@ -78,7 +77,6 @@ exports[`LogBoxContainer should render both an error and warning 1`] = ` } } onPressDismiss={[Function]} - onPressList={[Function]} onPressOpen={[Function]} totalLogCount={1} /> @@ -93,273 +91,53 @@ exports[`LogBoxContainer should render both an error and warning 1`] = ` `; -exports[`LogBoxContainer should render fatal before warn and error 1`] = ` - - - -`; - -exports[`LogBoxContainer should render fatal error even when disabled 1`] = ` - - - -`; - -exports[`LogBoxContainer should render most recent fatal 1`] = ` - - - -`; - -exports[`LogBoxContainer should render most recent syntax error 1`] = ` - - 199 | export default CrashReactApp; - | ^ - 200 |", - "fileName": "/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js", - "location": Object { - "column": 0, - "row": 199, - }, - }, - "componentStack": Array [], - "count": 1, - "level": "syntax", - "message": Object { - "content": "Should not be selected", - "substitutions": Array [], - }, - "stack": Array [], - "symbolicated": Object { - "error": null, - "stack": null, - "status": "NONE", - }, - }, - LogBoxLog { - "category": "Some kind of syntax error message", - "codeFrame": Object { - "content": " 197 | }); - 198 | -> 199 | export default CrashReactApp; - | ^ - 200 |", - "fileName": "/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js", - "location": Object { - "column": 0, - "row": 199, - }, - }, - "componentStack": Array [], - "count": 1, - "level": "syntax", - "message": Object { - "content": "Should be selected", - "substitutions": Array [], - }, - "stack": Array [], - "symbolicated": Object { - "error": null, - "stack": null, - "status": "NONE", - }, - }, - ] - } - onChangeSelectedIndex={[Function]} - onDismiss={[Function]} - onMinimize={[Function]} - selectedIndex={1} - /> - -`; - -exports[`LogBoxContainer should render null when disabled with non-fatal error and warning 1`] = `null`; - exports[`LogBoxContainer should render null with no logs 1`] = `null`; -exports[`LogBoxContainer should render syntax error even when disabled 1`] = ` +exports[`LogBoxContainer should render null with no selected log and disabled 1`] = `null`; + +exports[`LogBoxContainer should render selected fatal error even when disabled 1`] = ` + + + +`; + +exports[`LogBoxContainer should render selected syntax error even when disabled 1`] = ` `; -exports[`LogBoxContainer should render the latest error 1`] = ` +exports[`LogBoxContainer should render the latest error notification 1`] = ` @@ -470,7 +246,7 @@ exports[`LogBoxContainer should render the latest error 1`] = ` `; -exports[`LogBoxContainer should render the latest warning 1`] = ` +exports[`LogBoxContainer should render the latest warning notification 1`] = ` diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index f87422192fc..863d7529d2d 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -39,7 +39,7 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` onRetry={[Function]} /> @@ -87,6 +87,7 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` onRetry={[Function]} /> diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap index 862b32bc4cc..5ab79024164 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap @@ -1,243 +1,97 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`LogBoxInspectorFooter should render fatal button 1`] = ` +exports[`LogBoxInspectorFooter should render no buttons and a message for syntax error 1`] = ` - - - - Reload - - - Fatal - errors require a full reload - - - - + This error cannot be dismissed. + `; -exports[`LogBoxInspectorFooter should render fatal button for syntax erorr 1`] = ` +exports[`LogBoxInspectorFooter should render two buttons for error 1`] = ` - + - - - Reload - - - Syntax - errors require a full reload - - - - + text="Close" + /> `; -exports[`LogBoxInspectorFooter should render fatal for warning with a fatal 1`] = ` +exports[`LogBoxInspectorFooter should render two buttons for fatal 1`] = ` - + - - - Reload - - - Syntax - errors require a full reload - - - - + text="Close" + /> `; -exports[`LogBoxInspectorFooter should render two buttons for warning with no fatal 1`] = ` +exports[`LogBoxInspectorFooter should render two buttons for warning 1`] = ` `; +exports[`LogBoxInspectorHeader should render syntax error header 1`] = ` + + + + + Failed to compile + + + + +`; + exports[`LogBoxInspectorHeader should render two buttons for three or more total 1`] = `