From c1a433e544637cced2019f324b8487d73cbe3b6d Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 19 Jan 2026 07:29:14 -0800 Subject: [PATCH 1/3] Remove legacy component stack tests Summary: Remove tests that only test legacy component stack behavior. The 'stack' tests provide equivalent coverage for all scenarios. Legacy component stacks used a custom React string format (`\n in ComponentName (at filename.js:123)`) that was not symbolicated. The modern 'stack' format uses native JS Error stack frames that get properly symbolicated. This is the first step in removing `componentStackType` from LogBox entirely. The legacy parsing logic will be removed in a follow-up diff. Reviewed By: vzaidman Differential Revision: D90888272 --- .../LogBox/Data/__tests__/LogBoxLog-test.js | 260 -------- .../Data/__tests__/parseLogBoxLog-test.js | 591 +----------------- 2 files changed, 17 insertions(+), 834 deletions(-) diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js index 89fdd7d3607b94..fac2e7210e4cd5 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js @@ -40,29 +40,6 @@ const COMPONENT_CODE_FRAME: CodeCodeFrame = { content: 'Component', }; -// We can delete this when we delete legacy component stack types. -function getLogBoxLogLegacy() { - return new (require('../LogBoxLog').default)({ - level: 'warn', - isComponentError: false, - message: {content: '...', substitutions: []}, - stack: createStack(['A', 'B', 'C']), - category: 'Message category...', - componentStack: [ - { - content: 'LogBoxLog', - fileName: 'LogBoxLog.js', - location: {column: -1, row: 1}, - }, - ], - codeFrame: { - fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', - location: {row: 199, column: 0}, - content: '', - }, - }); -} - function getLogBoxLog() { return new (require('../LogBoxLog').default)({ level: 'warn', @@ -148,243 +125,6 @@ describe('LogBoxLog', () => { ); }); - describe('symbolicate legacy component stacks (no symbolication)', () => { - it('creates a LogBoxLog object', () => { - const log = getLogBoxLogLegacy(); - - expect(log.level).toEqual('warn'); - expect(log.message).toEqual({content: '...', substitutions: []}); - expect(log.stack).toEqual(createStack(['A', 'B', 'C'])); - expect(log.category).toEqual('Message category...'); - expect(log.componentStack).toEqual([ - { - content: 'LogBoxLog', - fileName: 'LogBoxLog.js', - location: {column: -1, row: 1}, - }, - ]); - expect(log.codeFrame).toEqual({ - fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', - location: {row: 199, column: 0}, - content: '', - }); - }); - - it('increments LogBoxLog count', () => { - const log = getLogBoxLogLegacy(); - - expect(log.count).toEqual(1); - - log.incrementCount(); - - expect(log.count).toEqual(2); - }); - - it('starts without a symbolicated stack', () => { - const log = getLogBoxLogLegacy(); - - expect(log.symbolicated).toEqual({ - error: null, - stack: null, - status: 'NONE', - }); - }); - - it('updates when symbolication is in progress', () => { - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.symbolicate(callback); - - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalledTimes(1); - expect(log.symbolicated).toEqual({ - error: null, - stack: null, - status: 'PENDING', - }); - - // Symbolicating while pending should not make more requests. - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - - log.symbolicate(callback); - expect(callback).not.toBeCalled(); - expect(getLogBoxSymbolication().symbolicate).not.toBeCalled(); - }); - - it('updates when symbolication finishes', async () => { - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.symbolicate(callback); - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalled(); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(2); - expect(callback).toBeCalledWith('COMPLETE'); - expect(log.symbolicated).toEqual({ - error: null, - stack: createStack(['S(A)', 'S(B)', 'S(C)']), - status: 'COMPLETE', - }); - - // Do not symbolicate again. - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - - log.symbolicate(callback); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(0); - expect(getLogBoxSymbolication().symbolicate).not.toBeCalled(); - }); - - it('updates when symbolication fails', async () => { - const error = new Error('...'); - getLogBoxSymbolication().symbolicate.mockImplementation(async stack => { - throw error; - }); - - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.symbolicate(callback); - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalled(); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(2); - expect(callback).toBeCalledWith('FAILED'); - expect(log.symbolicated).toEqual({ - error, - stack: null, - status: 'FAILED', - }); - - // Do not symbolicate again, retry if needed. - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - - log.symbolicate(callback); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(0); - expect(getLogBoxSymbolication().symbolicate).not.toBeCalled(); - }); - - it('retry updates when symbolication is in progress', () => { - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.retrySymbolicate(callback); - - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalledTimes(1); - expect(log.symbolicated).toEqual({ - error: null, - stack: null, - status: 'PENDING', - }); - - // Symbolicating while pending should not make more requests. - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - - log.symbolicate(callback); - expect(callback).not.toBeCalled(); - expect(getLogBoxSymbolication().symbolicate).not.toBeCalled(); - }); - - it('retry updates when symbolication finishes', async () => { - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.retrySymbolicate(callback); - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalled(); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(2); - expect(callback).toBeCalledWith('COMPLETE'); - expect(log.symbolicated).toEqual({ - error: null, - stack: createStack(['S(A)', 'S(B)', 'S(C)']), - status: 'COMPLETE', - }); - - // Do not symbolicate again - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - - log.retrySymbolicate(callback); - jest.runAllTicks(); - - expect(callback).toBeCalledTimes(0); - expect(getLogBoxSymbolication().symbolicate).not.toBeCalled(); - }); - - it('retry updates when symbolication fails', async () => { - const error = new Error('...'); - getLogBoxSymbolication().symbolicate.mockImplementation(async stack => { - throw error; - }); - - const log = getLogBoxLogLegacy(); - - const callback = jest.fn(); - log.retrySymbolicate(callback); - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalled(); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(2); - expect(callback).toBeCalledWith('FAILED'); - expect(log.symbolicated).toEqual({ - error, - stack: null, - status: 'FAILED', - }); - - // Retry to symbolicate again. - callback.mockClear(); - getLogBoxSymbolication().symbolicate.mockClear(); - getLogBoxSymbolication().symbolicate.mockImplementation(async stack => ({ - stack: createStack(stack.map(frame => `S(${frame.methodName})`)), - codeFrame: null, - })); - - log.retrySymbolicate(callback); - - expect(callback).toBeCalledTimes(1); - expect(callback).toBeCalledWith('PENDING'); - expect(getLogBoxSymbolication().symbolicate).toBeCalled(); - - await runMicrotasks(); - - expect(callback).toBeCalledTimes(2); - expect(callback).toBeCalledWith('COMPLETE'); - expect(log.symbolicated).toEqual({ - error: null, - stack: createStack(['S(A)', 'S(B)', 'S(C)']), - status: 'COMPLETE', - }); - }); - }); - describe('symbolicate component stacks', () => { it('creates a LogBoxLog object', () => { const log = getLogBoxLog(); diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index 298af1fff86cf7..a202ebf03fbd06 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -115,159 +115,6 @@ describe('parseLogBoxLog', () => { }); }); - it('does not duplicate message if component stack found but not parsed', () => { - expect( - parseLogBoxLog([ - 'Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.%s', - '\n\nCheck the render method of `MyOtherComponent`.', - '', - '\n in\n in\n in', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [], - category: - 'Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.', - message: { - content: - 'Each child in a list should have a unique "key" prop.\n\nCheck the render method of `MyOtherComponent`. See https://fb.me/react-warning-keys for more information.', - substitutions: [ - { - length: 48, - offset: 53, - }, - { - length: 0, - offset: 101, - }, - ], - }, - }); - }); - - it('detects a component stack in an interpolated warning', () => { - expect( - parseLogBoxLog([ - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s%s', - '\n\nCheck the render method of `Container(Component)`.', - '\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - category: - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s', - message: { - content: - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?\n\nCheck the render method of `Container(Component)`.', - substitutions: [ - { - length: 52, - offset: 120, - }, - ], - }, - }); - }); - - it('detects a component stack in the first argument', () => { - expect( - parseLogBoxLog([ - 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - - it('detects a component stack in the second argument', () => { - expect( - parseLogBoxLog([ - 'Some kind of message', - '\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - - it('detects a component stack in the nth argument', () => { - expect( - parseLogBoxLog([ - 'Some kind of message', - 'Some other kind of message', - '\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - 'Some third kind of message', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - category: - 'Some kind of message Some other kind of message Some third kind of message', - message: { - content: - 'Some kind of message Some other kind of message Some third kind of message', - substitutions: [], - }, - }); - }); - it('parses a transform error as a fatal', () => { const error: ExtendedExceptionData = { message: 'TransformError failed to transform file.', @@ -558,116 +405,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }); }); - it('parses an error log with `error.componentStack`', () => { - const error: ExtendedExceptionData = { - id: 0, - isFatal: false, - isComponentError: false, - message: '### Error', - originalMessage: '### Error', - name: '', - componentStackType: 'legacy', - componentStack: - '\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }; - - expect(parseLogBoxException(error)).toEqual({ - level: 'error', - category: '### Error', - isComponentError: false, - message: { - content: '### Error', - substitutions: [], - }, - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }); - }); - - it('parses an error log with a component stack in the message', () => { - const error: ExtendedExceptionData = { - id: 0, - isFatal: false, - isComponentError: false, - message: - 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - originalMessage: - 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', - name: '', - componentStackType: 'legacy', - componentStack: null, - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }; - expect(parseLogBoxException(error)).toEqual({ - level: 'error', - isComponentError: false, - stack: [ - { - collapse: false, - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - }, - ], - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: 'filename.js', - location: {column: -1, row: 1}, - }, - { - content: 'MyOtherComponent', - fileName: 'filename2.js', - location: {column: -1, row: 1}, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - it('parses a fatal exception', () => { const error: ExtendedExceptionData = { id: 0, @@ -800,269 +537,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }); }); - describe('Handles component stack frames without debug source', () => { - it('detects a component stack in an interpolated warning', () => { - expect( - parseLogBoxLog([ - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s%s', - '\n\nCheck the render method of `MyComponent`.', - '\n in MyComponent (created by MyOtherComponent)\n in MyOtherComponent (created by MyComponent)\n in MyAppComponent (created by MyOtherComponent)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - { - content: 'MyOtherComponent', - fileName: '', - location: null, - }, - { - content: 'MyAppComponent', - fileName: '', - location: null, - }, - ], - category: - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s', - message: { - content: - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?\n\nCheck the render method of `MyComponent`.', - substitutions: [ - { - length: 43, - offset: 120, - }, - ], - }, - }); - }); - - it('detects a component stack in the first argument', () => { - expect( - parseLogBoxLog([ - 'Some kind of message\n in MyComponent (created by MyOtherComponent)\n in MyOtherComponent (created by MyComponent)\n in MyAppComponent (created by MyOtherComponent)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - { - content: 'MyOtherComponent', - fileName: '', - location: null, - }, - { - content: 'MyAppComponent', - fileName: '', - location: null, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - - it('detects a component stack in the second argument', () => { - expect( - parseLogBoxLog([ - 'Some kind of message', - '\n in MyComponent (created by MyOtherComponent)\n in MyOtherComponent (created by MyComponent)\n in MyAppComponent (created by MyOtherComponent)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - { - content: 'MyOtherComponent', - fileName: '', - location: null, - }, - { - content: 'MyAppComponent', - fileName: '', - location: null, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - - it('detects a component stack in the nth argument', () => { - expect( - parseLogBoxLog([ - 'Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.%s', - '\n\nCheck the render method of `MyOtherComponent`.', - '', - '\n in MyComponent (created by MyOtherComponent)\n in MyOtherComponent (created by MyComponent)\n in MyAppComponent (created by MyOtherComponent)', - ]), - ).toEqual({ - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - { - content: 'MyOtherComponent', - fileName: '', - location: null, - }, - { - content: 'MyAppComponent', - fileName: '', - location: null, - }, - ], - category: - 'Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.', - message: { - content: - 'Each child in a list should have a unique "key" prop.\n\nCheck the render method of `MyOtherComponent`. See https://fb.me/react-warning-keys for more information.', - substitutions: [ - { - length: 48, - offset: 53, - }, - { - length: 0, - offset: 101, - }, - ], - }, - }); - }); - - it('detects a single component in a component stack', () => { - const error: ExtendedExceptionData = { - id: 0, - isFatal: true, - isComponentError: true, - message: - 'Error: Some kind of message\n\nThis error is located at:\n in MyComponent (created by MyOtherComponent)\n', - originalMessage: 'Some kind of message', - name: '', - componentStackType: 'legacy', - componentStack: '\n in MyComponent (created by MyOtherComponent)\n', - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }; - - expect(parseLogBoxException(error)).toEqual({ - level: 'fatal', - isComponentError: true, - stack: [ - { - collapse: false, - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - }, - ], - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - - it('parses an error log with `error.componentStack`', () => { - const error: ExtendedExceptionData = { - id: 0, - isFatal: false, - isComponentError: false, - message: '### Error', - originalMessage: '### Error', - name: '', - componentStack: - '\n in MyComponent (created by MyOtherComponent)\n in MyOtherComponent (created by MyComponent)\n in MyAppComponent (created by MyOtherComponent)', - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }; - - expect(parseLogBoxException(error)).toEqual({ - level: 'error', - category: '### Error', - isComponentError: false, - message: { - content: '### Error', - substitutions: [], - }, - componentStackType: 'legacy', - componentStack: [ - { - content: 'MyComponent', - fileName: '', - location: null, - }, - { - content: 'MyOtherComponent', - fileName: '', - location: null, - }, - { - content: 'MyAppComponent', - fileName: '', - location: null, - }, - ], - stack: [ - { - column: 1, - file: 'foo.js', - lineNumber: 1, - methodName: 'bar', - collapse: false, - }, - ], - }); - }); - }); - describe('Handles component stack frames formatted as call stacks in Hermes', () => { let originalHermesInternal; beforeEach(() => { @@ -1367,41 +841,45 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, it('detects a component stack for ts, tsx, jsx, and js files', () => { expect( parseLogBoxLog([ - 'Some kind of message\n in MyTSComponent (at MyTSXComponent.ts:1)\n in MyTSXComponent (at MyTSCComponent.tsx:1)\n in MyJSXComponent (at MyJSXComponent.jsx:1)\n in MyJSComponent (at MyJSComponent.js:1)', + 'Some kind of message\nMyTSComponent@/path/to/MyTSComponent.ts:1:1\nMyTSXComponent@/path/to/MyTSXComponent.tsx:2:1\nMyJSXComponent@/path/to/MyJSXComponent.jsx:3:1\nMyJSComponent@/path/to/MyJSComponent.js:4:1', ]), ).toEqual({ - componentStackType: 'legacy', + componentStackType: 'stack', componentStack: [ { + collapse: false, content: 'MyTSComponent', - fileName: 'MyTSXComponent.ts', + fileName: '/path/to/MyTSComponent.ts', location: { - column: -1, + column: 0, row: 1, }, }, { + collapse: false, content: 'MyTSXComponent', - fileName: 'MyTSCComponent.tsx', + fileName: '/path/to/MyTSXComponent.tsx', location: { - column: -1, - row: 1, + column: 0, + row: 2, }, }, { + collapse: false, content: 'MyJSXComponent', - fileName: 'MyJSXComponent.jsx', + fileName: '/path/to/MyJSXComponent.jsx', location: { - column: -1, - row: 1, + column: 0, + row: 3, }, }, { + collapse: false, content: 'MyJSComponent', - fileName: 'MyJSComponent.js', + fileName: '/path/to/MyJSComponent.js', location: { - column: -1, - row: 1, + column: 0, + row: 4, }, }, ], @@ -1413,41 +891,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }); }); - it('detects a component stack in the first argument (JSC)', () => { - expect( - parseLogBoxLog([ - 'Some kind of message\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', - ]), - ).toEqual({ - componentStackType: 'stack', - componentStack: [ - { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, - }, - { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, - }, - { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, - }, - ], - category: 'Some kind of message', - message: { - content: 'Some kind of message', - substitutions: [], - }, - }); - }); - it('detects a component stack in the second argument', () => { expect( parseLogBoxLog([ From f9c3df5cf4f2f6d2e0fe8250e7abe105d74e1b4b Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 19 Jan 2026 07:29:14 -0800 Subject: [PATCH 2/3] Remove componentStackType from source code Summary: All component stacks now use the native JS Error stack frame format which gets properly symbolicated. This removes the legacy/stack distinction since the legacy format (custom React string format) is no longer used. After this change, all component stacks will be symbolicated, improving the debugging experience. Reviewed By: vzaidman Differential Revision: D90888223 --- .../Libraries/LogBox/Data/LogBoxData.js | 3 - .../Libraries/LogBox/Data/LogBoxLog.js | 10 +- .../LogBox/Data/__tests__/LogBoxData-test.js | 1 - .../LogBox/Data/__tests__/LogBoxLog-test.js | 1 - .../Data/__tests__/parseLogBoxLog-test.js | 43 +------ .../Libraries/LogBox/Data/parseLogBoxLog.js | 121 ++++-------------- .../react-native/Libraries/LogBox/LogBox.js | 7 +- .../LogBoxInspector-test.js.snap | 2 - .../Libraries/LogBox/__tests__/LogBox-test.js | 2 - .../LogBoxInspectorContainer-test.js.snap | 4 - .../LogBoxNotificationContainer-test.js.snap | 2 - 11 files changed, 29 insertions(+), 167 deletions(-) diff --git a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js index 6d1f5cc79161e7..10d7b9a14a568c 100644 --- a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js +++ b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js @@ -13,7 +13,6 @@ import type {LogLevel} from './LogBoxLog'; import type { Category, ComponentStack, - ComponentStackType, ExtendedExceptionData, Message, } from './parseLogBoxLog'; @@ -32,7 +31,6 @@ export type LogData = Readonly<{ message: Message, category: Category, componentStack: ComponentStack, - componentStackType: ComponentStackType | null, stack?: string, }>; @@ -238,7 +236,6 @@ export function addLog(log: LogData): void { stack, category: log.category, componentStack: log.componentStack, - componentStackType: log.componentStackType || 'legacy', }), ); } catch (error: unknown) { diff --git a/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js b/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js index e68004677fe8db..1a9f56e98d1e2c 100644 --- a/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js +++ b/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js @@ -13,7 +13,6 @@ import type { Category, CodeFrame, ComponentStack, - ComponentStackType, Message, } from './parseLogBoxLog'; @@ -61,7 +60,6 @@ export type LogBoxLogData = Readonly<{ message: Message, stack: Stack, category: string, - componentStackType?: ComponentStackType, componentStack: ComponentStack, codeFrame?: ?CodeFrame, isComponentError: boolean, @@ -74,7 +72,6 @@ class LogBoxLog { type: ?string; category: Category; componentStack: ComponentStack; - componentStackType: ComponentStackType; stack: Stack; count: number; level: LogLevel; @@ -113,7 +110,6 @@ class LogBoxLog { this.stack = data.stack; this.category = data.category; this.componentStack = data.componentStack; - this.componentStackType = data.componentStackType || 'legacy'; this.codeFrame = data.codeFrame; this.isComponentError = data.isComponentError; this.extraData = data.extraData; @@ -132,9 +128,6 @@ class LogBoxLog { } getAvailableComponentStack(): ComponentStack { - if (this.componentStackType === 'legacy') { - return this.componentStack; - } return this.symbolicatedComponentStack.status === 'COMPLETE' ? this.symbolicatedComponentStack.componentStack : this.componentStack; @@ -180,7 +173,7 @@ class LogBoxLog { } if ( this.componentStack != null && - this.componentStackType === 'stack' && + this.componentStack.length > 0 && this.symbolicatedComponentStack.status !== 'PENDING' && this.symbolicatedComponentStack.status !== 'COMPLETE' ) { @@ -192,6 +185,7 @@ class LogBoxLog { data => { this.updateComponentStackStatus( null, + // TODO: we shouldn't need to convert back to ComponentStack any more convertStackToComponentStack(data.stack), data?.codeFrame, callback, diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index bd81e5b2a528c9..e7880ef8c384e6 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -65,7 +65,6 @@ const addLogs = (logs: Array, options: void | {flush: boolean}) => { }, category: message, componentStack: [], - componentStackType: null, }); if (options == null || options.flush !== false) { jest.runOnlyPendingTimers(); diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js index fac2e7210e4cd5..e3cb5f510e08b8 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js @@ -47,7 +47,6 @@ function getLogBoxLog() { message: {content: '...', substitutions: []}, stack: createStack(['A', 'B', 'C']), category: 'Message category...', - componentStackType: 'stack', componentStack: createComponentStack(['A', 'B', 'C']), codeFrame: null, }); diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index a202ebf03fbd06..e39da03d261ebb 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -21,7 +21,6 @@ const { describe('parseLogBoxLog', () => { it('parses strings', () => { expect(parseLogBoxLog(['A'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: 'A', message: { @@ -33,7 +32,6 @@ describe('parseLogBoxLog', () => { it('parses strings with arguments', () => { expect(parseLogBoxLog(['A', 'B', 'C'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: 'A B C', message: { @@ -45,7 +43,6 @@ describe('parseLogBoxLog', () => { it('parses formatted strings', () => { expect(parseLogBoxLog(['%s', 'A'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: '\ufeff%s', message: { @@ -62,7 +59,6 @@ describe('parseLogBoxLog', () => { it('parses formatted strings with insufficient arguments', () => { expect(parseLogBoxLog(['%s %s', 'A'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: '\ufeff%s %s', message: { @@ -83,7 +79,6 @@ describe('parseLogBoxLog', () => { it('parses formatted strings with excess arguments', () => { expect(parseLogBoxLog(['%s', 'A', 'B'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: '\ufeff%s B', message: { @@ -100,7 +95,6 @@ describe('parseLogBoxLog', () => { it('treats "%s" in arguments as literals', () => { expect(parseLogBoxLog(['%s', '%s', 'A'])).toEqual({ - componentStackType: 'legacy', componentStack: [], category: '\ufeff%s A', message: { @@ -121,7 +115,6 @@ describe('parseLogBoxLog', () => { originalMessage: 'TransformError failed to transform file.', name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -136,7 +129,6 @@ describe('parseLogBoxLog', () => { substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: 'TransformError failed to transform file.', }); @@ -160,7 +152,6 @@ describe('parseLogBoxLog', () => { 200 |`, name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -184,7 +175,6 @@ describe('parseLogBoxLog', () => { substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js-199-0', }); @@ -222,7 +212,6 @@ If you are sure the module exists, try these steps: \u001b[0m \u001b[90m 15 | \u001b[39m\u001b[36mimport\u001b[39m fbRemoteAsset from \u001b[32m'fbRemoteAsset'\u001b[39m\u001b[33m;\u001b[39m\u001b[0m`, name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -254,7 +243,6 @@ If you are sure the module exists, try these steps: substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: '/path/to/file.js-1-1', }); @@ -278,7 +266,6 @@ If you are sure the module exists, try these steps: 200 |`, name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -302,7 +289,6 @@ If you are sure the module exists, try these steps: substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js-199-0', }); @@ -328,7 +314,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets 200 |`, name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -354,7 +339,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js-1-1', }); @@ -376,7 +360,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets \u001b[0m \u001b[90m 46 | \u001b[39m headline\u001b[33m=\u001b[39m\u001b[32m"CrashReact Error Boundary"\u001b[39m\u001b[0m\n\u001b[0m \u001b[90m 47 | \u001b[39m body\u001b[33m=\u001b[39m{\u001b[32m\`\${this.state.errorMessage}\`\u001b[39m}\u001b[0m\n\u001b[0m\u001b[31m\u001b[1m>\u001b[22m\u001b[39m\u001b[90m 48 | \u001b[39m icon\u001b[33m=\u001b[39m{fbRemoteAsset(\u001b[32m'null_state_glyphs'\u001b[39m\u001b[33m,\u001b[39m {\u001b[0m\n\u001b[0m \u001b[90m | \u001b[39m \u001b[31m\u001b[1m^\u001b[22m\u001b[39m\u001b[0m\n\u001b[0m \u001b[90m 49 | \u001b[39m name\u001b[33m:\u001b[39m \u001b[32m'codexxx'\u001b[39m\u001b[33m,\u001b[39m\u001b[0m\n\u001b[0m \u001b[90m 50 | \u001b[39m size\u001b[33m:\u001b[39m \u001b[32m'112'\u001b[39m\u001b[33m,\u001b[39m\u001b[0m\n\u001b[0m \u001b[90m 51 | \u001b[39m })}\u001b[0m`, name: '', isComponentError: false, - componentStackType: 'legacy', componentStack: '', stack: ([]: Array), id: 0, @@ -399,7 +382,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, substitutions: [], }, stack: [], - componentStackType: 'legacy', componentStack: [], category: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js-1-1', }); @@ -412,7 +394,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, isComponentError: false, message: '### Fatal', originalMessage: '### Fatal', - componentStackType: 'legacy', componentStack: null, name: '', stack: [ @@ -434,7 +415,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, content: '### Fatal', substitutions: [], }, - componentStackType: 'legacy', componentStack: [], stack: [ { @@ -476,7 +456,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, content: '### Fatal', substitutions: [], }, - componentStackType: 'legacy', componentStack: [], stack: [ { @@ -523,7 +502,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, substitutions: [], }, isComponentError: false, - componentStackType: 'legacy', componentStack: [], stack: [ { @@ -558,7 +536,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -576,7 +553,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }, ], category: - 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s', + 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?\ufeff%s', message: { content: 'Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?\n\nCheck the render method of `MyComponent`.', @@ -596,7 +573,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, 'Some kind of message\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -604,8 +580,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, fileName: '/path/to/filename.js', location: {column: 1, row: 1}, }, - // TODO: we're missing the second component, - // because React isn't sending back a properly formatted stackframe. { collapse: false, content: 'MyAppComponent', @@ -628,7 +602,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -636,8 +609,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, fileName: '/path/to/filename.js', location: {column: 1, row: 1}, }, - // TODO: we're missing the second component, - // because React isn't sending back a properly formatted stackframe. { collapse: false, content: 'MyAppComponent', @@ -662,7 +633,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -680,7 +650,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }, ], category: - 'Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.', + 'Each child in a list should have a unique "key" prop.\ufeff%s\ufeff%s See https://fb.me/react-warning-keys for more information.', message: { content: 'Each child in a list should have a unique "key" prop.\n\nCheck the render method of `MyOtherComponent`. See https://fb.me/react-warning-keys for more information.', @@ -727,7 +697,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, content: '### Error', substitutions: [], }, - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -767,7 +736,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -809,7 +777,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, 'Some kind of message\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -844,7 +811,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, 'Some kind of message\nMyTSComponent@/path/to/MyTSComponent.ts:1:1\nMyTSXComponent@/path/to/MyTSXComponent.tsx:2:1\nMyJSXComponent@/path/to/MyJSXComponent.jsx:3:1\nMyJSComponent@/path/to/MyJSComponent.js:4:1', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -898,7 +864,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -936,7 +901,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, '\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', ]), ).toEqual({ - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -1005,7 +969,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, content: '### Error', substitutions: [], }, - componentStackType: 'stack', componentStack: [ { collapse: false, @@ -1048,7 +1011,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, originalMessage: 'Some kind of message\nMyComponent@/path/to/filename.js:1:2\nforEach@[native code]\nMyAppComponent@/path/to/app.js:100:20', name: '', - componentStackType: 'stack', componentStack: null, stack: [ { @@ -1072,7 +1034,6 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, methodName: 'bar', }, ], - componentStackType: 'stack', componentStack: [ { collapse: false, diff --git a/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js b/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js index 31a47205ebafe5..69fcc47cde8323 100644 --- a/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js @@ -19,10 +19,7 @@ import ansiRegex from 'ansi-regex'; const ANSI_REGEX = ansiRegex().source; const RE_TRANSFORM_ERROR = /^TransformError /; -const RE_COMPONENT_STACK_LINE = /\n {4}(in|at) /; -const RE_COMPONENT_STACK_LINE_GLOBAL = /\n {4}(in|at) /g; -const RE_COMPONENT_STACK_LINE_OLD = / {4}in/; -const RE_COMPONENT_STACK_LINE_NEW = / {4}at/; +const RE_COMPONENT_STACK_LINE = /\n {4}at/; const RE_COMPONENT_STACK_LINE_STACK_FRAME = /@.*\n/; // "TransformError " (Optional) and either "SyntaxError: " or "ReferenceError: " @@ -36,20 +33,6 @@ const RE_COMPONENT_STACK_LINE_STACK_FRAME = /@.*\n/; const RE_BABEL_TRANSFORM_ERROR_FORMAT = /^(?:TransformError )?(?:SyntaxError: |ReferenceError: )(.*): (.*) \((\d+):(\d+)\)\n\n([\s\S]+)/; -// Capturing groups: -// 1: component name -// "at" -// 2: file path including extension -// 3: line number -const RE_COMPONENT_STACK_WITH_SOURCE = - /(.*) \(at (.*\.(?:js|jsx|ts|tsx)):([\d]+)\)/; - -// Capturing groups: -// 1: component name -// "at" -// 2: parent component name -const RE_COMPONENT_STACK_NO_SOURCE = /(.*) \(created by .*\)/; - // Capturing groups: // - non-capturing "TransformError " (optional) // - non-capturing Error message @@ -132,7 +115,6 @@ export type Message = Readonly<{ }>; export type ComponentStack = ReadonlyArray; -export type ComponentStackType = 'legacy' | 'stack'; const SUBSTITUTION = UTFSequence.BOM + '%s'; @@ -212,75 +194,34 @@ export function parseInterpolation(args: ReadonlyArray): Readonly<{ } function isComponentStack(consoleArgument: string) { - const isOldComponentStackFormat = - RE_COMPONENT_STACK_LINE_OLD.test(consoleArgument); - const isNewComponentStackFormat = - RE_COMPONENT_STACK_LINE_NEW.test(consoleArgument); - const isNewJSCComponentStackFormat = - RE_COMPONENT_STACK_LINE_STACK_FRAME.test(consoleArgument); - + // Component stacks are formatted as call stack frames: + // - Hermes format: " at Component (/path/to/file.js:1:2)" + // - JSC format: "Component@/path/to/file.js:1:2" return ( - isOldComponentStackFormat || - isNewComponentStackFormat || - isNewJSCComponentStackFormat + RE_COMPONENT_STACK_LINE.test(consoleArgument) || + RE_COMPONENT_STACK_LINE_STACK_FRAME.test(consoleArgument) ); } export function parseComponentStack(message: string): { - type: ComponentStackType, stack: ComponentStack, } { - // In newer versions of React, the component stack is formatted as a call stack frame. - // First try to parse the component stack as a call stack frame, and if that doesn't - // work then we'll fallback to the old custom component stack format parsing. + // Component stacks are formatted as call stack frames. + // Parse the component stack using the error stack parser. const stack = parseErrorStack(message); - if (stack && stack.length > 0) { - return { - type: 'stack', - stack: stack.map(frame => ({ - content: frame.methodName, - collapse: frame.collapse || false, - fileName: frame.file == null ? 'unknown' : frame.file, - location: { - column: frame.column == null ? -1 : frame.column, - row: frame.lineNumber == null ? -1 : frame.lineNumber, - }, - })), - }; - } - const legacyStack = message - .split(RE_COMPONENT_STACK_LINE_GLOBAL) - .map(s => { - if (!s) { - return null; - } - const match = s.match(RE_COMPONENT_STACK_WITH_SOURCE); - if (match) { - let [content, fileName, row] = match.slice(1); - return { - content, - fileName, - location: {column: -1, row: parseInt(row, 10)}, - }; - } - - // In some cases, the component stack doesn't have a source. - const matchWithoutSource = s.match(RE_COMPONENT_STACK_NO_SOURCE); - if (matchWithoutSource) { - return { - content: matchWithoutSource[1], - fileName: '', - location: null, - }; - } - - return null; - }) - .filter(Boolean); - return { - type: 'legacy', - stack: legacyStack, + stack: + stack && stack.length > 0 + ? stack.map(frame => ({ + content: frame.methodName, + collapse: frame.collapse || false, + fileName: frame.file == null ? 'unknown' : frame.file, + location: { + column: frame.column == null ? -1 : frame.column, + row: frame.lineNumber == null ? -1 : frame.lineNumber, + }, + })) + : [], }; } @@ -300,7 +241,6 @@ export function parseLogBoxException( type: 'Metro Error', stack: [], isComponentError: false, - componentStackType: 'legacy', componentStack: [], codeFrame: { fileName, @@ -329,7 +269,6 @@ export function parseLogBoxException( level: 'syntax', stack: [], isComponentError: false, - componentStackType: 'legacy', componentStack: [], codeFrame: { fileName, @@ -360,7 +299,6 @@ export function parseLogBoxException( level: 'syntax', stack: [], isComponentError: false, - componentStackType: 'legacy', componentStack: [], codeFrame: { fileName, @@ -382,7 +320,6 @@ export function parseLogBoxException( level: 'syntax', stack: error.stack, isComponentError: error.isComponentError, - componentStackType: 'legacy', componentStack: [], message: { content: message, @@ -396,12 +333,11 @@ export function parseLogBoxException( const componentStack = error.componentStack; if (error.isFatal || error.isComponentError) { if (componentStack != null) { - const {type, stack} = parseComponentStack(componentStack); + const {stack} = parseComponentStack(componentStack); return { level: 'fatal', stack: error.stack, isComponentError: error.isComponentError, - componentStackType: type, componentStack: stack, extraData: error.extraData, ...parseInterpolation([message]), @@ -411,7 +347,6 @@ export function parseLogBoxException( level: 'fatal', stack: error.stack, isComponentError: error.isComponentError, - componentStackType: 'legacy', componentStack: [], extraData: error.extraData, ...parseInterpolation([message]), @@ -421,12 +356,11 @@ export function parseLogBoxException( if (componentStack != null) { // It is possible that console errors have a componentStack. - const {type, stack} = parseComponentStack(componentStack); + const {stack} = parseComponentStack(componentStack); return { level: 'error', stack: error.stack, isComponentError: error.isComponentError, - componentStackType: type, componentStack: stack, extraData: error.extraData, ...parseInterpolation([message]), @@ -458,14 +392,12 @@ export function withoutANSIColorStyles(message: unknown): unknown { export function parseLogBoxLog(args: ReadonlyArray): { componentStack: ComponentStack, - componentStackType: ComponentStackType, category: Category, message: Message, } { const message = withoutANSIColorStyles(args[0]); let argsWithoutComponentStack: Array = []; let componentStack: ComponentStack = []; - let componentStackType = 'legacy'; // Extract component stack from warnings like "Some warning%s". if ( @@ -477,9 +409,8 @@ export function parseLogBoxLog(args: ReadonlyArray): { if (typeof lastArg === 'string' && isComponentStack(lastArg)) { argsWithoutComponentStack = args.slice(0, -1); argsWithoutComponentStack[0] = message.slice(0, -2); - const {type, stack} = parseComponentStack(lastArg); + const {stack} = parseComponentStack(lastArg); componentStack = stack; - componentStackType = type; } } @@ -497,9 +428,8 @@ export function parseLogBoxLog(args: ReadonlyArray): { argsWithoutComponentStack.push(arg.slice(0, messageEndIndex)); } - const {type, stack} = parseComponentStack(arg); + const {stack} = parseComponentStack(arg); componentStack = stack; - componentStackType = type; } else { argsWithoutComponentStack.push(arg); } @@ -509,8 +439,5 @@ export function parseLogBoxLog(args: ReadonlyArray): { return { ...parseInterpolation(argsWithoutComponentStack), componentStack, - /* $FlowFixMe[incompatible-type] Natural Inference rollout. See - * https://fburl.com/workplace/6291gfvu */ - componentStackType, }; } diff --git a/packages/react-native/Libraries/LogBox/LogBox.js b/packages/react-native/Libraries/LogBox/LogBox.js index 1109e48b2ff565..86402ffa7c8bd6 100644 --- a/packages/react-native/Libraries/LogBox/LogBox.js +++ b/packages/react-native/Libraries/LogBox/LogBox.js @@ -166,7 +166,6 @@ if (__DEV__) { const result = parseLogBoxLog(args); const category = result.category; const message = result.message; - let componentStackType = result.componentStackType; let componentStack = result.componentStack; if ( (!componentStack || componentStack.length === 0) && @@ -177,7 +176,6 @@ if (__DEV__) { if (ownerStack != null && ownerStack.length > 0) { const parsedComponentStack = parseComponentStack(ownerStack); componentStack = parsedComponentStack.stack; - componentStackType = parsedComponentStack.type; } } if (!LogBoxData.isMessageIgnored(message.content)) { @@ -186,7 +184,6 @@ if (__DEV__) { category, message, componentStack, - componentStackType, }); } } catch (err: unknown) { @@ -221,8 +218,7 @@ if (__DEV__) { try { if (!isRCTLogAdviceWarning(...args)) { - const {category, message, componentStack, componentStackType} = - parseLogBoxLog(args); + const {category, message, componentStack} = parseLogBoxLog(args); if (!LogBoxData.isMessageIgnored(message.content)) { LogBoxData.addLog({ @@ -230,7 +226,6 @@ if (__DEV__) { category, message, componentStack, - componentStackType, }); } } diff --git a/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index f186bf65f738a6..bf3dae93c9646b 100644 --- a/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -22,7 +22,6 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` "category": "Some kind of message (third)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, @@ -80,7 +79,6 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` "category": "Some kind of message (first)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, diff --git a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js index 4a001a5a4df950..b0891c450241bd 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js +++ b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js @@ -273,7 +273,6 @@ describe('LogBox', () => { expect(LogBoxData.addLog).toBeCalledWith({ category: '%s ...', componentStack: [], - componentStackType: 'legacy', level: 'warn', message: { content: '(ADVICE) ...', @@ -306,7 +305,6 @@ describe('LogBox', () => { category: 'test', message: {content: 'Some warning', substitutions: []}, componentStack: [], - componentStackType: null, }); expect(LogBoxData.addLog).not.toHaveBeenCalled(); diff --git a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap index 697753cac0c590..c5a52601f16c7e 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap +++ b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap @@ -27,7 +27,6 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "category": "Some kind of message", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, @@ -72,7 +71,6 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "category": "Some kind of message (latest)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, @@ -139,7 +137,6 @@ exports[`LogBoxNotificationContainer should render the latest error notification "category": "Some kind of message (latest)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, @@ -198,7 +195,6 @@ exports[`LogBoxNotificationContainer should render the latest warning notificati "category": "Some kind of message (latest)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, diff --git a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap index d313325b71d5b8..92790141051c71 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap +++ b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap @@ -19,7 +19,6 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "category": "Some kind of message", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, @@ -46,7 +45,6 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "category": "Some kind of message (latest)", "codeFrame": undefined, "componentStack": Array [], - "componentStackType": "legacy", "count": 1, "extraData": undefined, "isComponentError": false, From dabf98ddb910de3993a29bbe58b16672d04f26c6 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 19 Jan 2026 07:29:14 -0800 Subject: [PATCH 3/3] =?UTF-8?q?Remove=20ComponentStack=20=E2=86=92=20Stack?= =?UTF-8?q?=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: LogBox was doing a wasteful round-trip conversion: parseComponentStack converted Stack → ComponentStack, stored it that way, then convertComponentStackToStack converted it back every time it was used. This eliminates the unnecessary conversion by storing componentStack as Stack directly from the start. The ComponentStack type is removed entirely since it's no longer needed. Also consolidates duplicate helper functions in LogBoxLog-test.js by removing createStackForComponentStack and createComponentStack, keeping only the createStack helper. Reviewed By: vzaidman Differential Revision: D90896380 --- .../Libraries/LogBox/Data/LogBoxData.js | 10 +- .../Libraries/LogBox/Data/LogBoxLog.js | 79 ++--- .../LogBox/Data/__tests__/LogBoxLog-test.js | 44 +-- .../Data/__tests__/parseLogBoxLog-test.js | 297 +++++++++--------- .../Libraries/LogBox/Data/parseLogBoxLog.js | 38 +-- .../react-native/Libraries/LogBox/LogBox.js | 33 +- .../LogBox/UI/LogBoxInspectorReactFrames.js | 10 +- .../LogBoxInspectorReactFrames-test.js | 77 +++-- .../LogBoxInspector-test.js.snap | 4 +- .../LogBoxInspectorContainer-test.js.snap | 8 +- .../LogBoxNotificationContainer-test.js.snap | 4 +- 11 files changed, 281 insertions(+), 323 deletions(-) diff --git a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js index 10d7b9a14a568c..de1ff3a5cc03c5 100644 --- a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js +++ b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js @@ -10,12 +10,8 @@ import type {ExtendedError} from '../../Core/ExtendedError'; import type {LogLevel} from './LogBoxLog'; -import type { - Category, - ComponentStack, - ExtendedExceptionData, - Message, -} from './parseLogBoxLog'; +import type {Stack} from './LogBoxSymbolication'; +import type {Category, ExtendedExceptionData, Message} from './parseLogBoxLog'; import DebuggerSessionObserver from '../../../src/private/devsupport/rndevtools/FuseboxSessionObserver'; import toExtendedError from '../../../src/private/utilities/toExtendedError'; @@ -30,7 +26,7 @@ export type LogData = Readonly<{ level: LogLevel, message: Message, category: Category, - componentStack: ComponentStack, + componentStack: Stack, stack?: string, }>; diff --git a/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js b/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js index 1a9f56e98d1e2c..08d3ac7d6c5b2d 100644 --- a/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js +++ b/packages/react-native/Libraries/LogBox/Data/LogBoxLog.js @@ -9,12 +9,7 @@ */ import type {Stack} from './LogBoxSymbolication'; -import type { - Category, - CodeFrame, - ComponentStack, - Message, -} from './parseLogBoxLog'; +import type {Category, CodeFrame, Message} from './parseLogBoxLog'; import * as LogBoxSymbolication from './LogBoxSymbolication'; @@ -22,45 +17,13 @@ type SymbolicationStatus = 'NONE' | 'PENDING' | 'COMPLETE' | 'FAILED'; export type LogLevel = 'warn' | 'error' | 'fatal' | 'syntax'; -// TODO: once component stacks are fully supported, we can refactor -// ComponentStack to just be Stack and remove these conversions fns. -function convertComponentStateToStack(componentStack: ComponentStack): Stack { - return componentStack.map(frame => ({ - column: frame?.location?.column, - file: frame.fileName, - lineNumber: frame?.location?.row, - methodName: frame.content, - collapse: false, - })); -} - -function convertStackToComponentStack(stack: Stack): ComponentStack { - const componentStack = []; - for (let i = 0; i < stack.length; i++) { - const frame = stack[i]; - // NOTE: Skip stack frames missing location. - if (frame.lineNumber != null && frame.column != null) { - componentStack.push({ - fileName: frame?.file || '', - location: { - row: frame.lineNumber, - column: frame.column, - }, - content: frame.methodName, - collapse: false, - }); - } - } - return componentStack; -} - export type LogBoxLogData = Readonly<{ level: LogLevel, type?: ?string, message: Message, stack: Stack, category: string, - componentStack: ComponentStack, + componentStack: Stack, codeFrame?: ?CodeFrame, isComponentError: boolean, extraData?: unknown, @@ -71,7 +34,7 @@ class LogBoxLog { message: Message; type: ?string; category: Category; - componentStack: ComponentStack; + componentStack: Stack; stack: Stack; count: number; level: LogLevel; @@ -89,16 +52,16 @@ class LogBoxLog { status: 'NONE', }; symbolicatedComponentStack: - | Readonly<{error: null, componentStack: null, status: 'NONE'}> - | Readonly<{error: null, componentStack: null, status: 'PENDING'}> + | Readonly<{error: null, stack: null, status: 'NONE'}> + | Readonly<{error: null, stack: null, status: 'PENDING'}> | Readonly<{ error: null, - componentStack: ComponentStack, + stack: Stack, status: 'COMPLETE', }> - | Readonly<{error: Error, componentStack: null, status: 'FAILED'}> = { + | Readonly<{error: Error, stack: null, status: 'FAILED'}> = { error: null, - componentStack: null, + stack: null, status: 'NONE', }; onNotificationPress: ?() => void; @@ -127,9 +90,9 @@ class LogBoxLog { : this.stack; } - getAvailableComponentStack(): ComponentStack { + getAvailableComponentStack(): Stack { return this.symbolicatedComponentStack.status === 'COMPLETE' - ? this.symbolicatedComponentStack.componentStack + ? this.symbolicatedComponentStack.stack : this.componentStack; } @@ -140,9 +103,7 @@ class LogBoxLog { retry = true; } if (this.symbolicatedComponentStack.status !== 'COMPLETE') { - LogBoxSymbolication.deleteStack( - convertComponentStateToStack(this.componentStack), - ); + LogBoxSymbolication.deleteStack(this.componentStack); retry = true; } if (retry) { @@ -178,15 +139,11 @@ class LogBoxLog { this.symbolicatedComponentStack.status !== 'COMPLETE' ) { this.updateComponentStackStatus(null, null, null, callback); - const componentStackFrames = convertComponentStateToStack( - this.componentStack, - ); - LogBoxSymbolication.symbolicate(componentStackFrames, []).then( + LogBoxSymbolication.symbolicate(this.componentStack, []).then( data => { this.updateComponentStackStatus( null, - // TODO: we shouldn't need to convert back to ComponentStack any more - convertStackToComponentStack(data.stack), + data.stack, data?.codeFrame, callback, ); @@ -236,7 +193,7 @@ class LogBoxLog { updateComponentStackStatus( error: ?Error, - componentStack: ?ComponentStack, + stack: ?Stack, codeFrame: ?CodeFrame, callback?: (status: SymbolicationStatus) => void, ): void { @@ -244,22 +201,22 @@ class LogBoxLog { if (error != null) { this.symbolicatedComponentStack = { error, - componentStack: null, + stack: null, status: 'FAILED', }; - } else if (componentStack != null) { + } else if (stack != null) { if (codeFrame) { this.componentCodeFrame = codeFrame; } this.symbolicatedComponentStack = { error: null, - componentStack, + stack, status: 'COMPLETE', }; } else { this.symbolicatedComponentStack = { error: null, - componentStack: null, + stack: null, status: 'PENDING', }; } diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js index e3cb5f510e08b8..8c3901e4ed4041 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js @@ -12,7 +12,6 @@ import type {SymbolicatedStackTrace} from '../../../Core/Devtools/symbolicateStackTrace'; import type {StackFrame} from '../../../Core/NativeExceptionsManager'; -import type {CodeFrame} from '../parseLogBoxLog'; jest.mock('../LogBoxSymbolication', () => { return {__esModule: true, symbolicate: jest.fn(), deleteStack: jest.fn()}; @@ -69,7 +68,7 @@ const createStack = (methodNames: Array) => methodName, })); -const createStackForComponentStack = (methodNames: Array) => +const createComponentStack = (methodNames: Array) => methodNames.map((methodName): StackFrame => ({ column: 0, file: 'file://path/to/component.js', @@ -77,17 +76,6 @@ const createStackForComponentStack = (methodNames: Array) => methodName, })); -const createComponentStack = (methodNames: Array) => - methodNames.map((methodName): CodeFrame => ({ - collapse: false, - content: methodName, - location: { - row: 1, - column: 0, - }, - fileName: 'file://path/to/component.js', - })); - function mockSymbolicate( stack: ReadonlyArray, stackCodeFrame: ?CodeCodeFrame, @@ -100,9 +88,7 @@ function mockSymbolicate( firstFrame.file.indexOf('component.js') > 0 ) { return { - stack: createStackForComponentStack( - stack.map(frame => `C(${frame.methodName})`), - ), + stack: createStack(stack.map(frame => `C(${frame.methodName})`)), codeFrame: COMPONENT_CODE_FRAME, }; } @@ -157,7 +143,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: null, + stack: null, status: 'NONE', }); }); @@ -179,7 +165,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: null, + stack: null, status: 'PENDING', }); @@ -216,7 +202,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); @@ -266,7 +252,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); @@ -318,7 +304,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error, - componentStack: null, + stack: null, status: 'FAILED', }); @@ -351,7 +337,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: null, + stack: null, status: 'PENDING', }); @@ -388,7 +374,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); @@ -432,7 +418,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error, - componentStack: null, + stack: null, status: 'FAILED', }); @@ -463,7 +449,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); @@ -502,7 +488,7 @@ describe('LogBoxLog', () => { }); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); @@ -534,7 +520,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); @@ -574,7 +560,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error, - componentStack: null, + stack: null, status: 'FAILED', }); @@ -605,7 +591,7 @@ describe('LogBoxLog', () => { expect(log.codeFrame).toBe(STACK_CODE_FRAME); expect(log.symbolicatedComponentStack).toEqual({ error: null, - componentStack: createComponentStack(['C(A)', 'C(B)', 'C(C)']), + stack: createStack(['C(A)', 'C(B)', 'C(C)']), status: 'COMPLETE', }); expect(log.componentCodeFrame).toBe(COMPONENT_CODE_FRAME); diff --git a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index e39da03d261ebb..fc9905f1e75eef 100644 --- a/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -538,18 +538,18 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, // TODO: we're missing the second component, // because React isn't sending back a properly formatted stackframe. { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: @@ -575,16 +575,18 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, + // TODO: we're missing the second component, + // because React isn't sending back a properly formatted stackframe. { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: 'Some kind of message', @@ -604,16 +606,18 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, + // TODO: we're missing the second component, + // because React isn't sending back a properly formatted stackframe. { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: 'Some kind of message', @@ -635,18 +639,18 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, // TODO: we're missing the second component, // because React isn't sending back a properly formatted stackframe. { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: @@ -699,20 +703,21 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }, componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, // TODO: we're missing the second component, // because React isn't sending back a properly formatted stackframe. { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], + extraData: undefined, stack: [ { column: 1, @@ -738,22 +743,25 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: @@ -779,22 +787,25 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: 'Some kind of message', @@ -813,40 +824,32 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyTSComponent', - fileName: '/path/to/MyTSComponent.ts', - location: { - column: 0, - row: 1, - }, + arguments: [], + methodName: 'MyTSComponent', + file: '/path/to/MyTSComponent.ts', + lineNumber: 1, + column: 0, }, { - collapse: false, - content: 'MyTSXComponent', - fileName: '/path/to/MyTSXComponent.tsx', - location: { - column: 0, - row: 2, - }, + arguments: [], + methodName: 'MyTSXComponent', + file: '/path/to/MyTSXComponent.tsx', + lineNumber: 2, + column: 0, }, { - collapse: false, - content: 'MyJSXComponent', - fileName: '/path/to/MyJSXComponent.jsx', - location: { - column: 0, - row: 3, - }, + arguments: [], + methodName: 'MyJSXComponent', + file: '/path/to/MyJSXComponent.jsx', + lineNumber: 3, + column: 0, }, { - collapse: false, - content: 'MyJSComponent', - fileName: '/path/to/MyJSComponent.js', - location: { - column: 0, - row: 4, - }, + arguments: [], + methodName: 'MyJSComponent', + file: '/path/to/MyJSComponent.js', + lineNumber: 4, + column: 0, }, ], category: 'Some kind of message', @@ -866,22 +869,25 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: 'Some kind of message', @@ -903,22 +909,25 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ).toEqual({ componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], category: @@ -971,24 +980,28 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }, componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], + extraData: undefined, stack: [ { column: 1, @@ -1036,24 +1049,28 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, ], componentStack: [ { - collapse: false, - content: 'MyComponent', - fileName: '/path/to/filename.js', - location: {column: 1, row: 1}, + arguments: [], + methodName: 'MyComponent', + file: '/path/to/filename.js', + lineNumber: 1, + column: 1, }, { - collapse: false, - content: 'forEach', - fileName: '[native code]', - location: {column: -1, row: -1}, + arguments: [], + methodName: 'forEach', + file: '[native code]', + lineNumber: null, + column: null, }, { - collapse: false, - content: 'MyAppComponent', - fileName: '/path/to/app.js', - location: {column: 19, row: 100}, + arguments: [], + methodName: 'MyAppComponent', + file: '/path/to/app.js', + lineNumber: 100, + column: 19, }, ], + extraData: undefined, category: 'Some kind of message', message: { content: 'Some kind of message', diff --git a/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js b/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js index 69fcc47cde8323..730bcc6156f8d8 100644 --- a/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js @@ -10,6 +10,7 @@ import type {ExceptionData} from '../../Core/NativeExceptionsManager'; import type {LogBoxLogData} from './LogBoxLog'; +import type {Stack} from './LogBoxSymbolication'; import parseErrorStack from '../../Core/Devtools/parseErrorStack'; import UTFSequence from '../../UTFSequence'; @@ -76,19 +77,11 @@ const RE_BABEL_CODE_FRAME_MARKER_PATTERN = new RegExp( 'm', ); -export function hasComponentStack(args: ReadonlyArray): boolean { - for (const arg of args) { - if (typeof arg === 'string' && isComponentStack(arg)) { - return true; - } - } - return false; -} - -export type ExtendedExceptionData = ExceptionData & { +export type ExtendedExceptionData = Readonly<{ + ...ExceptionData, isComponentError: boolean, ... -}; +}>; export type Category = string; export type CodeFrame = Readonly<{ content: string, @@ -114,8 +107,6 @@ export type Message = Readonly<{ >, }>; -export type ComponentStack = ReadonlyArray; - const SUBSTITUTION = UTFSequence.BOM + '%s'; export function parseInterpolation(args: ReadonlyArray): Readonly<{ @@ -204,24 +195,11 @@ function isComponentStack(consoleArgument: string) { } export function parseComponentStack(message: string): { - stack: ComponentStack, + stack: Stack, } { - // Component stacks are formatted as call stack frames. - // Parse the component stack using the error stack parser. const stack = parseErrorStack(message); return { - stack: - stack && stack.length > 0 - ? stack.map(frame => ({ - content: frame.methodName, - collapse: frame.collapse || false, - fileName: frame.file == null ? 'unknown' : frame.file, - location: { - column: frame.column == null ? -1 : frame.column, - row: frame.lineNumber == null ? -1 : frame.lineNumber, - }, - })) - : [], + stack: stack ?? [], }; } @@ -391,13 +369,13 @@ export function withoutANSIColorStyles(message: unknown): unknown { } export function parseLogBoxLog(args: ReadonlyArray): { - componentStack: ComponentStack, + componentStack: Stack, category: Category, message: Message, } { const message = withoutANSIColorStyles(args[0]); let argsWithoutComponentStack: Array = []; - let componentStack: ComponentStack = []; + let componentStack: Stack = []; // Extract component stack from warnings like "Some warning%s". if ( diff --git a/packages/react-native/Libraries/LogBox/LogBox.js b/packages/react-native/Libraries/LogBox/LogBox.js index 86402ffa7c8bd6..7bf123fdaa7fc5 100644 --- a/packages/react-native/Libraries/LogBox/LogBox.js +++ b/packages/react-native/Libraries/LogBox/LogBox.js @@ -9,6 +9,7 @@ */ import type {IgnorePattern, LogData} from './Data/LogBoxData'; +import type {Stack} from './Data/LogBoxSymbolication'; import type {ExtendedExceptionData} from './Data/parseLogBoxLog'; import toExtendedError from '../../src/private/utilities/toExtendedError'; @@ -16,6 +17,33 @@ import Platform from '../Utilities/Platform'; import RCTLog from '../Utilities/RCTLog'; import * as React from 'react'; +// TODO: Remove support for LegacyComponentStackFrame in a future version. +// This is kept for backward compatibility with external callers of LogBox.addLog. +function convertLegacyComponentStack(componentStack: Stack): Stack { + if (componentStack.length === 0) { + return []; + } + // Detect legacy format by checking for 'content' property + const firstFrame = componentStack[0]; + if ( + firstFrame != null && + typeof firstFrame === 'object' && + // $FlowExpectedError[prop-missing] + typeof firstFrame.content === 'string' + ) { + // Convert from legacy ComponentStack to Stack format + return (componentStack: $FlowFixMe).map(frame => ({ + methodName: frame.content, + lineNumber: frame.location.row, + column: frame.location.column, + file: frame.fileName, + collapse: frame.collapse ?? false, + })); + } + // Already in the new Stack format + return componentStack; +} + export type {LogData, ExtendedExceptionData, IgnorePattern}; let LogBox; @@ -137,7 +165,10 @@ if (__DEV__) { addLog(log: LogData): void { if (isLogBoxInstalled) { - LogBoxData.addLog(log); + LogBoxData.addLog({ + ...log, + componentStack: convertLegacyComponentStack(log.componentStack), + }); } }, diff --git a/packages/react-native/Libraries/LogBox/UI/LogBoxInspectorReactFrames.js b/packages/react-native/Libraries/LogBox/UI/LogBoxInspectorReactFrames.js index 5ab475c12560d3..f4310452ebf058 100644 --- a/packages/react-native/Libraries/LogBox/UI/LogBoxInspectorReactFrames.js +++ b/packages/react-native/Libraries/LogBox/UI/LogBoxInspectorReactFrames.js @@ -95,9 +95,9 @@ function LogBoxInspectorReactFrames(props: Props): React.Node { // Older versions of DevTools do not provide full path. // This will not work on Windows, remove check once the // DevTools return the full file path. - frame.fileName.startsWith('/') + frame.file != null && frame.file.startsWith('/') ? () => - openFileInEditor(frame.fileName, frame.location?.row ?? 1) + openFileInEditor(frame.file ?? '', frame.lineNumber ?? 1) : null } style={componentStyles.frame}> @@ -106,13 +106,13 @@ function LogBoxInspectorReactFrames(props: Props): React.Node { id="logbox_component_stack_frame_text" style={componentStyles.frameName}> {'<'} - {frame.content} + {frame.methodName} {' />'} - {getPrettyFileName(frame.fileName)} - {frame.location ? `:${frame.location.row}` : ''} + {frame.file != null ? getPrettyFileName(frame.file) : ''} + {frame.lineNumber != null ? `:${frame.lineNumber}` : ''} diff --git a/packages/react-native/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js b/packages/react-native/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js index ad6dc12fe36731..e116ce6021279c 100644 --- a/packages/react-native/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js +++ b/packages/react-native/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js @@ -65,12 +65,11 @@ describe('LogBoxInspectorReactFrames', () => { category: 'Some kind of message', componentStack: [ { - content: 'MyComponent', - fileName: 'MyComponentFile.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent', + file: 'MyComponentFile.js', + lineNumber: 1, + column: -1, + collapse: false, }, ], }) @@ -96,12 +95,11 @@ describe('LogBoxInspectorReactFrames', () => { category: 'Some kind of message', componentStack: [ { - content: 'MyComponent', - fileName: '/path/to/MyComponentFile.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent', + file: '/path/to/MyComponentFile.js', + lineNumber: 1, + column: -1, + collapse: false, }, ], }) @@ -127,12 +125,11 @@ describe('LogBoxInspectorReactFrames', () => { category: 'Some kind of message', componentStack: [ { - content: 'MyComponent', - fileName: '/path/to/index.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent', + file: '/path/to/index.js', + lineNumber: 1, + column: -1, + collapse: false, }, ], }) @@ -158,36 +155,32 @@ describe('LogBoxInspectorReactFrames', () => { category: 'Some kind of message', componentStack: [ { - content: 'MyComponent', - fileName: '/path/to/index.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent', + file: '/path/to/index.js', + lineNumber: 1, + column: -1, + collapse: false, }, { - content: 'MyComponent2', - fileName: '/path/to/index2.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent2', + file: '/path/to/index2.js', + lineNumber: 1, + column: -1, + collapse: false, }, { - content: 'MyComponent3', - fileName: '/path/to/index3.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent3', + file: '/path/to/index3.js', + lineNumber: 1, + column: -1, + collapse: false, }, { - content: 'MyComponent4', - fileName: '/path/to/index4.js', - location: { - row: 1, - column: -1, - }, + methodName: 'MyComponent4', + file: '/path/to/index4.js', + lineNumber: 1, + column: -1, + collapse: false, }, ], }) diff --git a/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index bf3dae93c9646b..e1ffa1836f24a5 100644 --- a/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/packages/react-native/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -38,8 +38,8 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, @@ -95,8 +95,8 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, diff --git a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap index c5a52601f16c7e..a7717078b1d9db 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap +++ b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap @@ -43,8 +43,8 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, @@ -87,8 +87,8 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, @@ -153,8 +153,8 @@ exports[`LogBoxNotificationContainer should render the latest error notification "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, @@ -211,8 +211,8 @@ exports[`LogBoxNotificationContainer should render the latest warning notificati "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, diff --git a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap index 92790141051c71..3f28e4a45d93f1 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap +++ b/packages/react-native/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap @@ -35,8 +35,8 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined, @@ -61,8 +61,8 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "status": "NONE", }, "symbolicatedComponentStack": Object { - "componentStack": null, "error": null, + "stack": null, "status": "NONE", }, "type": undefined,