Skip to content

Commit bdcfd41

Browse files
emily8rownfacebook-github-bot
authored andcommitted
Skip observer notifications during performance tracing
Summary: During performance tracing, LogBoxStateSubscription was causing an unnecessary re-render even though LogBox messages are already dropped. This happened because async work (e.g. symbolication completing) could trigger `handleUpdate()` during a trace, notifying observers and causing `LogBoxStateSubscription` to `setState` and re-render. The fix adds an `isTracing()` guard inside `handleUpdate()`'s `setImmediate` callback so observer notifications are skipped for the entire duration of a trace. When tracing ends, a new `handleUpdate()` call syncs observers with the current (cleared) state. This eliminates a spurious update visible in Marketplace sell items page performance traces. Differential Revision: D94075642
1 parent e76a6e2 commit bdcfd41

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

packages/react-native/Libraries/LogBox/Data/LogBoxData.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ function handleUpdate(): void {
130130
if (updateTimeout == null) {
131131
updateTimeout = setImmediate(() => {
132132
updateTimeout = null;
133+
if (TracingStateObserver.isTracing()) {
134+
return;
135+
}
133136
const nextState = getNextState();
134137
observers.forEach(({observer}) => observer(nextState));
135138
});
@@ -212,6 +215,8 @@ export function addLog(log: LogData): void {
212215
isTracing => {
213216
if (isTracing) {
214217
clear();
218+
} else {
219+
handleUpdate();
215220
}
216221
},
217222
);

packages/react-native/Libraries/LogBox/Data/__tests__/LogBoxData-test.js

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,5 +853,73 @@ describe('LogBoxData', () => {
853853
LogBoxDataWithMock.observe(observerAfter).unsubscribe();
854854
expect(Array.from(observerAfter.mock.calls[0][0].logs).length).toBe(0);
855855
});
856+
857+
it('does not notify observers while tracing is active', () => {
858+
const LogBoxDataWithMock = require('../LogBoxData');
859+
860+
LogBoxDataWithMock.addLog({
861+
level: 'warn',
862+
message: {content: 'Log 1', substitutions: []},
863+
category: 'log-1',
864+
componentStack: [],
865+
});
866+
jest.runOnlyPendingTimers();
867+
868+
const observer = jest.fn();
869+
const subscription = LogBoxDataWithMock.observe(observer);
870+
871+
// observe() calls the observer once synchronously on subscribe
872+
expect(observer).toHaveBeenCalledTimes(1);
873+
874+
// Start tracing
875+
mockIsTracing = jest.fn(() => true);
876+
if (mockSubscribeCallback) {
877+
mockSubscribeCallback(true);
878+
}
879+
jest.runOnlyPendingTimers();
880+
881+
// Observer should NOT have been called again during tracing
882+
expect(observer).toHaveBeenCalledTimes(1);
883+
884+
subscription.unsubscribe();
885+
});
886+
887+
it('notifies observers when tracing ends to sync state', () => {
888+
const LogBoxDataWithMock = require('../LogBoxData');
889+
890+
LogBoxDataWithMock.addLog({
891+
level: 'warn',
892+
message: {content: 'Log 1', substitutions: []},
893+
category: 'log-1',
894+
componentStack: [],
895+
});
896+
jest.runOnlyPendingTimers();
897+
898+
const observer = jest.fn();
899+
const subscription = LogBoxDataWithMock.observe(observer);
900+
901+
// observe() calls once on subscribe
902+
expect(observer).toHaveBeenCalledTimes(1);
903+
expect(Array.from(observer.mock.calls[0][0].logs).length).toBe(1);
904+
905+
// Start tracing — clears logs but does not notify
906+
mockIsTracing = jest.fn(() => true);
907+
if (mockSubscribeCallback) {
908+
mockSubscribeCallback(true);
909+
}
910+
jest.runOnlyPendingTimers();
911+
expect(observer).toHaveBeenCalledTimes(1);
912+
913+
// End tracing — should notify observer with cleared state
914+
mockIsTracing = jest.fn(() => false);
915+
if (mockSubscribeCallback) {
916+
mockSubscribeCallback(false);
917+
}
918+
jest.runOnlyPendingTimers();
919+
expect(observer).toHaveBeenCalledTimes(2);
920+
expect(Array.from(observer.mock.calls[1][0].logs).length).toBe(0);
921+
922+
subscription.unsubscribe();
923+
});
856924
});
857925
});

0 commit comments

Comments
 (0)