Introduce shallow serialization

Summary:
Changelog: [Flipper] Improve serialisation mechanism format & speed

The default serialisation mechanism used by Flipper to serialise plugin states is very flexible, taking care of maps, sets, dates etc. However, it is also really slow, leading to issues like in the related tasks, and work arounds like D17402443 (98bc01618f) to skip the whole process for plugins.

This diff changes the serialisation mechanism to have a better trade off between speed and convenience: For now we will only apply the smart serialisation for objects living at the _root_ of the serialised object, but it won't be applied recursively.

This sounds like a dangerous change, but works well in practice:
* I went through all `persistedState` and `createState` definition (the types), and the idea that complex types like Map and Set only live at the root of the persisted state holds up nicely. That makes sense as well since plugins typically store literally the same data as that they have received over the wire, except that they put it in some maps, sets etc.
* I introduced `assertSerializable` that only runs in dev/test, which will check (recursively, but without all the cloning) to see if a tree is indeed serialisable.
* The fact that by swapping this mechanism rarely existing unit test for exportData needed changes proves that the assumption that only roots are relevant generally upholds (or that plugin authors don't write enough tests ;-)).
* I verified that popular plugins still import / export correctly (actually *more* plugins are exportable now than before, thanks to sandy wrapper introduced earlier)

Reviewed By: jknoxville

Differential Revision: D29327499

fbshipit-source-id: 0ff17d9c5eb68fccfc2937b634cfa8f4f924247d

desktop/app/src/plugin.tsx

@@ -12,13 +12,18 @@ import {Logger} from './fb-interfaces/Logger';
 import Client from './Client';
 import {Component} from 'react';
 import BaseDevice from './devices/BaseDevice';
-import {serialize, deserialize} from './utils/serialization';
 import {StaticView} from './reducers/connections';
 import {State as ReduxState} from './reducers';
 import {DEFAULT_MAX_QUEUE_SIZE} from './reducers/pluginMessageQueue';
 import {ActivatablePluginDetails} from 'flipper-plugin-lib';
 import {Settings} from './reducers/settings';
-import {Notification, Idler, _SandyPluginDefinition} from 'flipper-plugin';
+import {
+  Notification,
+  Idler,
+  _SandyPluginDefinition,
+  _makeShallowSerializable,
+  _deserializeShallowObject,
+} from 'flipper-plugin';
 
 type Parameters = {[key: string]: any};
 
@@ -145,24 +150,44 @@ export abstract class FlipperBasePlugin<
     statusUpdate?: (msg: string) => void,
     idler?: Idler,
     pluginName?: string,
-  ) => Promise<string> = (
+  ) => Promise<string> = async (
     persistedState: StaticPersistedState,
-    statusUpdate?: (msg: string) => void,
-    idler?: Idler,
-    pluginName?: string,
+    _statusUpdate?: (msg: string) => void,
+    _idler?: Idler,
+    _pluginName?: string,
   ) => {
-    return serialize(
-      persistedState,
-      idler,
-      statusUpdate,
-      pluginName != null ? `Serializing ${pluginName}` : undefined,
+    if (
+      persistedState &&
+      typeof persistedState === 'object' &&
+      !Array.isArray(persistedState)
+    ) {
+      return JSON.stringify(
+        Object.fromEntries(
+          Object.entries(persistedState).map(([key, value]) => [
+            key,
+            _makeShallowSerializable(value), // make first level of persisted state serializable
+          ]),
+        ),
       );
+    } else {
+      return JSON.stringify(persistedState);
+    }
   };
 
   static deserializePersistedState: (
     serializedString: string,
   ) => StaticPersistedState = (serializedString: string) => {
-    return deserialize(serializedString);
+    const raw = JSON.parse(serializedString);
+    if (raw && typeof raw === 'object' && !Array.isArray(raw)) {
+      return Object.fromEntries(
+        Object.entries(raw).map(([key, value]) => [
+          key,
+          _deserializeShallowObject(value),
+        ]),
+      );
+    } else {
+      return raw;
+    }
   };
 
   teardown(): void {}

desktop/app/src/utils/exportData.tsx

@@ -49,7 +49,6 @@ import {getPluginTitle, isSandyPlugin} from './pluginUtils';
 import {capture} from './screenshot';
 import {uploadFlipperMedia} from '../fb-stubs/user';
 import {Idler} from 'flipper-plugin';
-import {deserializeObject, makeObjectSerializable} from './serialization';
 
 export const IMPORT_FLIPPER_TRACE_EVENT = 'import-flipper-trace';
 export const EXPORT_FLIPPER_TRACE_EVENT = 'export-flipper-trace';
@@ -264,15 +263,14 @@ async function exportSandyPluginStates(
       if (!res[client.id]) {
         res[client.id] = {};
       }
-      // makeObjectSerializable is slow but very convenient by default. If people want to speed things up
-      res[client.id][pluginId] = await makeObjectSerializable(
-        await client.sandyPluginStates
+      try {
+        res[client.id][pluginId] = await client.sandyPluginStates
           .get(pluginId)!
-          .exportState(idler, statusUpdate),
-        idler,
-        statusUpdate,
-        'Serializing plugin: ' + pluginId,
-      );
+          .exportState(idler, statusUpdate);
+      } catch (error) {
+        console.error('Error while serializing plugin ' + pluginId, error);
+        throw new Error(`Failed to serialize plugin ${pluginId}: ${error}`);
+      }
     }
   }
   return res;
@@ -461,11 +459,10 @@ export async function processStore(
       idler,
     );
 
-    const devicePluginStates = await makeObjectSerializable(
-      await device.exportState(idler, statusUpdate, selectedPlugins),
+    const devicePluginStates = await device.exportState(
       idler,
       statusUpdate,
-      'Serializing device plugins',
+      selectedPlugins,
     );
 
     statusUpdate('Uploading screenshot...');
@@ -800,7 +797,7 @@ export function importDataToStore(source: string, data: string, store: Store) {
   archivedDevice.loadDevicePlugins(
     store.getState().plugins.devicePlugins,
     store.getState().connections.enabledDevicePlugins,
-    deserializeObject(device.pluginStates),
+    device.pluginStates,
   );
   store.dispatch({
     type: 'REGISTER_DEVICE',
@@ -829,9 +826,7 @@ export function importDataToStore(source: string, data: string, store: Store) {
   });
 
   clients.forEach((client: {id: string; query: ClientQuery}) => {
-    const sandyPluginStates = deserializeObject(
-      json.pluginStates2[client.id] || {},
-    );
+    const sandyPluginStates = json.pluginStates2[client.id] || {};
     const clientPlugins: Array<string> = [
       ...keys
         .filter((key) => {

desktop/flipper-plugin/package.json

@@ -22,7 +22,6 @@
     "uuid": "^8.3.2"
   },
   "devDependencies": {
-    "@types/jest": "^26.0.23",
     "@types/string-natural-compare": "^3.0.0",
     "jest-mock-console": "^1.1.0",
     "typescript": "^4.3.4"

desktop/flipper-plugin/src/index.ts

@@ -125,6 +125,10 @@ export {
   ElementID,
 } from './ui/elements-inspector/ElementsInspector';
 export {useMemoize} from './utils/useMemoize';
+export {
+  makeShallowSerializable as _makeShallowSerializable,
+  deserializeShallowObject as _deserializeShallowObject,
+} from './utils/shallowSerialization';
 
 export {createTablePlugin} from './utils/createTablePlugin';
 

desktop/flipper-plugin/src/plugin/PluginBase.tsx

@@ -390,10 +390,13 @@ export abstract class BasePluginInstance {
 
   private serializeRootStates() {
     return Object.fromEntries(
-      Object.entries(this.rootStates).map(([key, atom]) => [
-        key,
-        atom.serialize(),
-      ]),
+      Object.entries(this.rootStates).map(([key, atom]) => {
+        try {
+          return [key, atom.serialize()];
+        } catch (e) {
+          throw new Error(`Failed to serialize state '${key}': ${e}`);
+        }
+      }),
     );
   }
 

desktop/flipper-plugin/src/state/atom.tsx

@@ -10,6 +10,10 @@
 import {produce, Draft, enableMapSet} from 'immer';
 import {useState, useEffect} from 'react';
 import {Persistable, registerStorageAtom} from '../plugin/PluginBase';
+import {
+  deserializeShallowObject,
+  makeShallowSerializable,
+} from '../utils/shallowSerialization';
 
 enableMapSet();
 
@@ -46,11 +50,11 @@ class AtomValue<T> implements Atom<T>, Persistable {
   }
 
   deserialize(value: T) {
-    this.set(value);
+    this.set(deserializeShallowObject(value));
   }
 
   serialize() {
-    return this.get();
+    return makeShallowSerializable(this.get());
   }
 
   update(recipe: (draft: Draft<T>) => void) {

desktop/flipper-plugin/src/utils/__tests__/shallowSerialization.node.tsx

@@ -0,0 +1,249 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {
+  makeShallowSerializable,
+  deserializeShallowObject,
+} from '../shallowSerialization';
+import mockConsole from 'jest-mock-console';
+
+class TestObject extends Object {
+  constructor(title: Object, map?: Map<any, any>, set?: Set<any>) {
+    super();
+    this.title = title;
+    this.map = map;
+    this.set = set;
+  }
+  title: Object;
+  map?: Map<any, any>;
+  set?: Set<any>;
+}
+
+test('test cyclic data structure', () => {
+  const a: any = {x: 0, b: {c: []}};
+  a.b.c.push(a);
+  expect(() => {
+    makeShallowSerializable(a);
+  }).toThrowErrorMatchingInlineSnapshot(
+    `"Cycle detected: object at path '.b.c.0' is referring to itself: '[object Object]'"`,
+  );
+});
+
+test('test shared data structure', () => {
+  const restoreConsole = mockConsole();
+  try {
+    const a = {hello: 'world'};
+    const b = {x: a, y: a};
+
+    const res = JSON.parse(JSON.stringify(makeShallowSerializable(b)));
+    expect(res).toEqual({
+      x: {hello: 'world'},
+      y: {hello: 'world'},
+    });
+    expect(b.x).toBe(b.y);
+    expect(res.x).not.toBe(res.y);
+    // @ts-ignore
+    expect(console.warn.mock.calls).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "Duplicate value, object lives at path '.y', but also at path '.x': '[object Object]'. This might not behave correct after import and lead to unnecessary big exports.",
+        ],
+      ]
+    `);
+  } finally {
+    restoreConsole();
+  }
+});
+
+test('test makeObjectSerializable function for unnested object with no Set and Map', () => {
+  const obj = {key1: 'value1', key2: 'value2'};
+  const output = makeShallowSerializable(obj);
+  expect(output).toEqual(obj);
+
+  // Testing numbers
+  const obj2 = {key1: 1, key2: 2};
+  const output2 = makeShallowSerializable(obj2);
+  expect(output2).toEqual(obj2);
+});
+
+test('makeObjectSerializable function for unnested object with values which returns false when put in an if condition', () => {
+  const obj2 = {key1: 0, key2: ''};
+  const output2 = makeShallowSerializable(obj2);
+  return expect(output2).toEqual(obj2);
+});
+
+test('test deserializeShallowObject function for unnested object with no Set and Map', () => {
+  const obj = {key1: 'value1', key2: 'value2'};
+  const output = deserializeShallowObject(obj);
+  expect(output).toEqual(obj);
+
+  // Testing numbers
+  const obj2 = {key1: 1, key2: 2};
+  const output2 = deserializeShallowObject(obj2);
+  expect(output2).toEqual(obj2);
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for nested object with no Set and Map', () => {
+  const subObj = {key1: 'value1', key2: 'value2'};
+  const subObj2 = {key21: 'value21', key22: 'value22'};
+  const obj = {key1: subObj, key2: subObj2};
+  const output = makeShallowSerializable(obj);
+  expect(output).toEqual(obj);
+  expect(deserializeShallowObject(output)).toEqual(obj);
+
+  const subObjNum = {key1: 1, key2: 2};
+  const subObjNum2 = {key21: 21, key22: 22};
+  const obj2 = {key1: subObjNum, key2: subObjNum2};
+  const output2 = makeShallowSerializable(obj2);
+  expect(output2).toEqual(obj2);
+  expect(deserializeShallowObject(output2)).toEqual(obj2);
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for Map and Set with no nesting', () => {
+  const map = new Map([
+    ['k1', 'v1'],
+    ['k2', 'v2'],
+  ]);
+  const output = makeShallowSerializable(map);
+  const expected = {
+    __flipper_object_type__: 'Map',
+    data: [
+      ['k1', 'v1'],
+      ['k2', 'v2'],
+    ],
+  };
+  expect(output).toEqual(expected);
+  expect(deserializeShallowObject(output)).toEqual(map);
+
+  const set = new Set([1, 2, 3, 4, 5, 6, 5, 4, 3, 2, 1]);
+  const outputSet = makeShallowSerializable(set);
+  const expectedSet = {
+    __flipper_object_type__: 'Set',
+    data: [1, 2, 3, 4, 5, 6],
+  };
+  expect(outputSet).toEqual(expectedSet);
+  expect(deserializeShallowObject(outputSet)).toEqual(set);
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for Map and Set with nesting', () => {
+  const map = new Map([
+    [{title: 'k1'}, {title: 'v1'}],
+    [{title: 'k2'}, {title: 'v2'}],
+  ]);
+  const output = makeShallowSerializable(map);
+  const expected = {
+    __flipper_object_type__: 'Map',
+    data: [
+      [{title: 'k1'}, {title: 'v1'}],
+      [{title: 'k2'}, {title: 'v2'}],
+    ],
+  };
+  expect(output).toEqual(expected);
+  expect(deserializeShallowObject(output)).toEqual(map);
+
+  const set = new Set([
+    {title: '1'},
+    {title: '2'},
+    {title: '3'},
+    {title: '4'},
+    {title: '5'},
+    {title: '6'},
+  ]);
+  const outputSet = makeShallowSerializable(set);
+  const expectedSet = {
+    __flipper_object_type__: 'Set',
+    data: [
+      {title: '1'},
+      {title: '2'},
+      {title: '3'},
+      {title: '4'},
+      {title: '5'},
+      {title: '6'},
+    ],
+  };
+  expect(outputSet).toEqual(expectedSet);
+  expect(deserializeShallowObject(outputSet)).toEqual(set);
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for custom Object', () => {
+  const obj = new TestObject('title');
+  expect(() => {
+    makeShallowSerializable(obj);
+  }).toThrowErrorMatchingInlineSnapshot(
+    `"Unserializable object type (TestObject) at path '.': [object Object]."`,
+  );
+});
+
+test('test makeObjectSerializable and deserializeShallowObject object with map', () => {
+  const nestedObjWithMap = {
+    map: new Map([
+      ['k1', 'v1'],
+      ['k2', 'v2'],
+    ]),
+  };
+  expect(() => {
+    makeShallowSerializable(nestedObjWithMap);
+  }).toThrowErrorMatchingInlineSnapshot(
+    `"Unserializable object type (Map) at path '.map': [object Map]."`,
+  );
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for Array as input', () => {
+  const arr = [1, 2, 4, 5];
+  const output = makeShallowSerializable(arr);
+  expect(output).toEqual(arr);
+  expect(deserializeShallowObject(output)).toEqual(arr);
+
+  const arrMap = [
+    new Map([
+      ['a1', 'v1'],
+      ['a2', 'v2'],
+    ]),
+  ];
+
+  expect(() => {
+    makeShallowSerializable(arrMap);
+  }).toThrowErrorMatchingInlineSnapshot(
+    `"Unserializable object type (Map) at path '.0': [object Map]."`,
+  );
+});
+
+test('test serialize and deserializeShallowObject function for non Object input', () => {
+  expect(makeShallowSerializable('octopus')).toEqual('octopus');
+  expect(deserializeShallowObject(makeShallowSerializable('octopus'))).toEqual(
+    'octopus',
+  );
+  expect(makeShallowSerializable(24567)).toEqual(24567);
+  expect(deserializeShallowObject(makeShallowSerializable(24567))).toEqual(
+    24567,
+  );
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for Date input', () => {
+  const date = new Date(2021, 1, 29, 10, 31, 7, 205);
+  expect(makeShallowSerializable(date)).toMatchInlineSnapshot(`
+    Object {
+      "__flipper_object_type__": "Date",
+      "data": 1614555067205,
+    }
+  `);
+  expect(deserializeShallowObject(makeShallowSerializable(date))).toEqual(date);
+});
+
+test('test makeObjectSerializable and deserializeShallowObject function for Map of Sets', () => {
+  const map = new Map([
+    ['k1', new Set([1, 2, 3, 4, 5, 6])],
+    [new Set([1, 2]), new Map([['k3', 'v3']])],
+  ] as any);
+  expect(() => {
+    makeShallowSerializable(map);
+  }).toThrowErrorMatchingInlineSnapshot(
+    `"Unserializable object type (Set) at path '.01': [object Set]."`,
+  );
+});

desktop/flipper-plugin/src/utils/shallowSerialization.tsx

@@ -0,0 +1,187 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+/**
+ * makeShallowSerializable will prepare common data structures, like Map and Set, for JSON serialization.
+ * However, this will happen only for the root object and not recursively to keep things efficiently.
+ *
+ * The function does not take care of actual stringification; use JSON.serialize.
+ */
+export function makeShallowSerializable(obj: any): any {
+  if (!obj || typeof obj !== 'object') {
+    assertSerializable(obj);
+    return obj;
+  }
+  if (obj instanceof Map) {
+    const data = Array.from(obj.entries());
+    assertSerializable(data);
+    return {
+      __flipper_object_type__: 'Map',
+      data,
+    };
+  } else if (obj instanceof Set) {
+    const data = Array.from(obj.values());
+    assertSerializable(data);
+    return {
+      __flipper_object_type__: 'Set',
+      data,
+    };
+  } else if (obj instanceof Date) {
+    return {
+      __flipper_object_type__: 'Date',
+      data: obj.getTime(),
+    };
+  } else {
+    assertSerializable(obj);
+    return obj;
+  }
+}
+
+/**
+ * Inverse of makeShallowSerializable
+ */
+export function deserializeShallowObject(obj: any): any {
+  if (!obj || typeof obj !== 'object') {
+    return obj;
+  }
+  if (obj['__flipper_object_type__']) {
+    const type = obj['__flipper_object_type__'];
+    switch (type) {
+      case 'Map': {
+        return new Map(obj.data);
+      }
+      case 'Set': {
+        return new Set(obj.data);
+      }
+      case 'Date':
+        return new Date(obj.data);
+    }
+  }
+  return obj;
+}
+
+/**
+ * Asserts a value is JSON serializable.
+ * Will print a warning if a value is JSON serializable, but isn't a pure tree
+ */
+export function assertSerializable(obj: any) {
+  if (
+    process.env.NODE_ENV !== 'test' &&
+    process.env.NODE_ENV !== 'development'
+  ) {
+    return;
+  }
+  // path to current object
+  const path: string[] = [];
+  // current object stack
+  const stack = new Set<any>();
+  // past objects, object -> path to reach it
+  const seen = new Set<any>();
+
+  // to safe a lot of memory allocations, if we find a duplicate, we just start over again to search for the first,
+  // rather than storing all paths at first encounter
+  let duplicateFound = false;
+  let duplicatePath: string[] | undefined;
+  let duplicateObject: any = undefined;
+  let done = false;
+
+  function check(value: any) {
+    if (value === null || done) {
+      return;
+    }
+    switch (typeof value) {
+      case 'undefined':
+        // undefined is not strictly speaking serializable, but behaves fine.
+        // JSON.stringify({x : undefined})  ==>   '{}'
+        break;
+      case 'boolean':
+      case 'number':
+      case 'string':
+        break;
+      case 'object':
+        // A cycle is truly not serializable, as it would create an unending serialization loop...
+        if (stack.has(value)) {
+          throw new Error(
+            `Cycle detected: object at path '.${path.join(
+              '.',
+            )}' is referring to itself: '${value}'`,
+          );
+        }
+        // Encountering an object multiple times is bad, as reference equality will be lost upon
+        // deserialization, so the data isn't properly normalised.
+        // But it *might* work fine, and can serialize, so we just warn
+
+        // Warning is only printed during the second check loop, so that we know *both* paths
+        // - Second walk (which finds first object)
+        if (duplicateFound && duplicateObject && value === duplicateObject) {
+          console.warn(
+            `Duplicate value, object lives at path '.${duplicatePath!.join(
+              '.',
+            )}', but also at path '.${path!.join(
+              '.',
+            )}': '${value}'. This might not behave correct after import and lead to unnecessary big exports.`,
+          );
+          done = true; // no need to finish the second walk
+          break;
+        }
+        // - First walk (which detects the duplicate and stores location of duplicate)
+        if (!duplicateFound) {
+          if (seen.has(value)) {
+            duplicateFound = true;
+            duplicateObject = value;
+            duplicatePath = path.slice();
+          }
+          seen.add(value);
+        }
+        stack.add(value);
+        const proto = Object.getPrototypeOf(value);
+        if (Array.isArray(value)) {
+          value.forEach((child, index) => {
+            path.push('' + index);
+            check(child);
+            path.pop();
+          });
+        } else if (proto === null || proto === Object.prototype) {
+          for (const key in value) {
+            path.push(key);
+            check(value[key]);
+            path.pop();
+          }
+        } else {
+          throw new Error(
+            `Unserializable object type (${
+              proto?.constructor?.name ?? 'Unknown'
+            }) at path '.${path.join('')}': ${value}.`,
+          );
+        }
+        stack.delete(value);
+        break;
+      case 'bigint':
+      case 'function':
+      case 'symbol':
+      default:
+        throw new Error(
+          `Unserializable value (${typeof value}) at path '.${path.join(
+            '.',
+          )}': '${value}'`,
+        );
+    }
+  }
+
+  check(obj);
+  // if there is a duplicate found, re-walk the tree so that we can print both of the paths and report it
+  // this setup is slightly more confusion in code than walking once and storing past paths,
+  // but a lot more efficient :)
+  if (duplicateFound) {
+    path.splice(0);
+    seen.clear();
+    stack.clear();
+    check(obj);
+  }
+}

docs/extending/flipper-plugin.mdx

@@ -473,7 +473,7 @@ Optionally, `options` can be provided when creating state. Supported options:
 
 * `persist: string`. If the `persist` value is set, this state container will be serialized when n Flipper snapshot export is being made. When a snapshot is imported into Flipper, and plugins are initialized, this state container will load its initial value from the snapshot, rather than using the `initialValue` parameter. The `persist` key should be unique within the plugin and only be set if the state stored in this container is serializable and won't become unreasonably large. See also `exportState` and `initialState` in the [`TestUtils`](#testutils) section.
 
-Serializable is defined as: non-cyclic data, consisting purely of primitive values, plain objects, arrays or Date, Set or Map objects.
+Serializable is defined as: non-cyclic data, consisting purely of primitive values, plain objects and arrays. Precisely as the root, `Date`, `Set` or `Map` objects are allowed as well, but they shouldn't appear deeper in the tree.
 
 #### The state atom object