Added some more test coverage; reverted rename

facebook · May 15, 2020 · 9761ee5 · 9761ee5
1 parent d83e15a
commit 9761ee5
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 21 deletions.
diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js
@@ -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();
   });
 });
diff --git a/packages/react-devtools-shared/src/__tests__/profilingUtils-test.js b/packages/react-devtools-shared/src/__tests__/profilingUtils-test.js
@@ -16,7 +16,7 @@ describe('profiling utils', () => {
 
   it('should throw if importing older/unsupported data', () => {
     expect(() =>
-      utils.prepareProfilingDataFrontendForImport(
+      utils.prepareProfilingDataFrontendFromExport(
         ({
           version: 0,
           dataForRoots: [],

diff --git a/packages/react-devtools-shared/src/__tests__/utils.js b/packages/react-devtools-shared/src/__tests__/utils.js
@@ -173,14 +173,15 @@ export function requireTestRenderer(): ReactTestRenderer {
 export function exportImportHelper(bridge: FrontendBridge, store: Store): void {
   const {
     prepareProfilingDataExport,
-    prepareProfilingDataFrontendForImport,
+    prepareProfilingDataFrontendFromExport,
   } = require('react-devtools-shared/src/devtools/views/Profiler/utils');
 
   const {profilerStore} = store;
 
   expect(profilerStore.profilingData).not.toBeNull();
 
   const profilingDataFrontendInitial = ((profilerStore.profilingData: any): ProfilingDataFrontend);
+  expect(profilingDataFrontendInitial.imported).toBe(false);
 
   const profilingDataExport = prepareProfilingDataExport(
     profilingDataFrontendInitial,
@@ -194,20 +195,21 @@ export function exportImportHelper(bridge: FrontendBridge, store: Store): void {
   );
   const parsedProfilingDataExport = JSON.parse(serializedProfilingDataExport);
 
-  const profilingDataFrontendImport = prepareProfilingDataFrontendForImport(
+  const profilingDataFrontend = prepareProfilingDataFrontendFromExport(
     (parsedProfilingDataExport: any),
   );
+  expect(profilingDataFrontend.imported).toBe(true);
 
   // Sanity check that profiling snapshots are serialized correctly.
   expect(profilingDataFrontendInitial.dataForRoots).toEqual(
-    profilingDataFrontendImport.dataForRoots,
+    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-imported data so tests can re-run assertions.
-    profilerStore.profilingData = profilingDataFrontendImport;
+    profilerStore.profilingData = profilingDataFrontend;
   });
 }
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js
@@ -138,16 +138,16 @@ function ProfilerContextController({children}: Props) {
       selectFiberName(name);
 
       // Sync selection to the Components tab for convenience.
+      // 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
       ) {
-        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) {
+        // 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,

diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilingImportExportButtons.js b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilingImportExportButtons.js
@@ -16,7 +16,7 @@ import ButtonIcon from '../ButtonIcon';
 import {StoreContext} from '../context';
 import {
   prepareProfilingDataExport,
-  prepareProfilingDataFrontendForImport,
+  prepareProfilingDataFrontendFromExport,
 } from './utils';
 import {downloadFile} from '../utils';
 
@@ -80,7 +80,7 @@ export default function ProfilingImportExportButtons() {
           const profilingDataExport = ((JSON.parse(
             raw,
           ): any): ProfilingDataExport);
-          profilerStore.profilingData = prepareProfilingDataFrontendForImport(
+          profilerStore.profilingData = prepareProfilingDataFrontendFromExport(
             profilingDataExport,
           );
         } catch (error) {

diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js b/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js
@@ -103,7 +103,7 @@ export function prepareProfilingDataFrontendFromBackendAndStore(
 }
 
 // Converts a Profiling data export into the format required by the Store.
-export function prepareProfilingDataFrontendForImport(
+export function prepareProfilingDataFrontendFromExport(
   profilingDataExport: ProfilingDataExport,
 ): ProfilingDataFrontend {
   const {version} = profilingDataExport;