Skip to content

Commit

Permalink
Profiler: Skip reading element for imported data (#18913)
Browse files Browse the repository at this point in the history
* skip reading element for imported data

* rename nodes & enable store lookup for components tab

* replace names

* Added some more test coverage; reverted rename

Co-authored-by: Brian Vaughn <bvaughn@fb.com>
  • Loading branch information
bl00mber and Brian Vaughn committed May 15, 2020
1 parent 7c08090 commit dd2e36d
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -337,18 +337,10 @@ describe('ProfilerContext', () => {
await utils.actAsync(() => context.selectFiber(parentID, 'Parent'));
expect(selectedElementID).toBe(parentID);

// We expect a "no element found" warning.
// Let's hide it from the test console though.
spyOn(console, 'warn');

// Select an unmounted element and verify no Components tab selection doesn't change.
await utils.actAsync(() => context.selectFiber(childID, 'Child'));
expect(selectedElementID).toBe(parentID);

expect(console.warn).toHaveBeenCalledWith(
`No element found with id "${childID}"`,
);

done();
});
});
8 changes: 6 additions & 2 deletions packages/react-devtools-shared/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export function exportImportHelper(bridge: FrontendBridge, store: Store): void {
expect(profilerStore.profilingData).not.toBeNull();

const profilingDataFrontendInitial = ((profilerStore.profilingData: any): ProfilingDataFrontend);
expect(profilingDataFrontendInitial.imported).toBe(false);

const profilingDataExport = prepareProfilingDataExport(
profilingDataFrontendInitial,
Expand All @@ -197,15 +198,18 @@ export function exportImportHelper(bridge: FrontendBridge, store: Store): void {
const profilingDataFrontend = prepareProfilingDataFrontendFromExport(
(parsedProfilingDataExport: any),
);
expect(profilingDataFrontend.imported).toBe(true);

// Sanity check that profiling snapshots are serialized correctly.
expect(profilingDataFrontendInitial).toEqual(profilingDataFrontend);
expect(profilingDataFrontendInitial.dataForRoots).toEqual(
profilingDataFrontend.dataForRoots,
);

// Snapshot the JSON-parsed object, rather than the raw string, because Jest formats the diff nicer.
expect(parsedProfilingDataExport).toMatchSnapshot('imported data');

act(() => {
// Apply the new exported-then-reimported data so tests can re-run assertions.
// Apply the new exported-then-imported data so tests can re-run assertions.
profilerStore.profilingData = profilingDataFrontend;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,20 +138,24 @@ function ProfilerContextController({children}: Props) {
selectFiberName(name);

// Sync selection to the Components tab for convenience.
if (id !== null) {
const element = store.getElementByID(id);

// Keep in mind that profiling data may be from a previous session.
// In that case, IDs may match up arbitrarily; to be safe, compare both ID and display name.
if (element !== null && element.displayName === name) {
// Keep in mind that profiling data may be from a previous session.
// If data has been imported, we should skip the selection sync.
if (
id !== null &&
profilingData !== null &&
profilingData.imported === false
) {
// We should still check to see if this element is still in the store.
// It may have been removed during profiling.
if (store.containsElement(id)) {
dispatch({
type: 'SELECT_ELEMENT_BY_ID',
payload: id,
});
}
}
},
[dispatch, selectFiberID, selectFiberName, store],
[dispatch, selectFiberID, selectFiberName, store, profilingData],
);

const setRootIDAndClearFiber = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export type ProfilingDataForRootFrontend = {|
export type ProfilingDataFrontend = {|
// Profiling data per root.
dataForRoots: Map<number, ProfilingDataForRootFrontend>,
imported: boolean,
|};

export type CommitDataExport = {|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function prepareProfilingDataFrontendFromBackendAndStore(
);
});

return {dataForRoots};
return {dataForRoots, imported: false};
}

// Converts a Profiling data export into the format required by the Store.
Expand Down Expand Up @@ -156,7 +156,7 @@ export function prepareProfilingDataFrontendFromExport(
},
);

return {dataForRoots};
return {dataForRoots, imported: true};
}

// Converts a Store Profiling data into a format that can be safely (JSON) serialized for export.
Expand Down

0 comments on commit dd2e36d

Please sign in to comment.