From d16155dfd9286725f091eda5dbc964ed4c8bafbd Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 13:30:20 -0500 Subject: [PATCH 01/46] zulipVersion [nfc]: Put methods on the prototype. --- src/utils/zulipVersion.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/utils/zulipVersion.js b/src/utils/zulipVersion.js index 58607f46ab1..78d4c6b49a4 100644 --- a/src/utils/zulipVersion.js +++ b/src/utils/zulipVersion.js @@ -39,17 +39,21 @@ export class ZulipVersion { /** * The raw version string that was passed to the constructor. */ - raw = () => this._raw; + raw() { + return this._raw; + } /** * Data to be sent to Sentry to help with event aggregation. */ - elements = () => this._elements; + elements() { + return this._elements; + } /** * True if this version is later than or equal to a given threshold. */ - isAtLeast = (otherZulipVersion: string | ZulipVersion) => { + isAtLeast(otherZulipVersion: string | ZulipVersion) { const otherZulipVersionInstance = otherZulipVersion instanceof ZulipVersion ? otherZulipVersion @@ -66,12 +70,12 @@ export class ZulipVersion { // It's a tie so far, and one of the arrays has ended. The array with // further elements wins. return this._comparisonArray.length >= otherComparisonArray.length; - }; + } /** * Parse the raw string into a VersionElements. */ - static _getElements = (raw: string): VersionElements => { + static _getElements(raw: string): VersionElements { const result: VersionElements = { major: undefined, minor: undefined, @@ -110,12 +114,12 @@ export class ZulipVersion { } return result; - }; + } /** * Compute a number[] to be used in .isAtLeast comparisons. */ - static _getComparisonArray = (elements: VersionElements): number[] => { + static _getComparisonArray(elements: VersionElements): number[] { const { major, minor, patch, flag, numCommits } = elements; const result: number[] = []; @@ -149,5 +153,5 @@ export class ZulipVersion { } return result; - }; + } } From 4e833a4089351141f5ad831b98e508891518e5bd Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 12:59:15 -0500 Subject: [PATCH 02/46] replaceRevive: Copy remotedev-serialize's tests, commented out. We'll soon be inlining remotedev-serialize, and we'll want to at least maintain the same test coverage the code had when it lived there. At first, the tests will just import remotedev-serialize's code and run that; then, we'll switch it over to testing that code as it appears in our project. Start with the tests and snapshots commented out; then, once we've made a few changes in the next few commits, they'll be able to run. --- .prettierignore | 1 + .../replaceRevive-test.js.snap | 23 +++ src/boot/__tests__/replaceRevive-test.js | 150 ++++++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap create mode 100644 src/boot/__tests__/replaceRevive-test.js diff --git a/.prettierignore b/.prettierignore index 0c903f42751..4f2feccbe55 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,3 +3,4 @@ src/emoji/__tests__/data-test.js # We're not allowing Prettier to touch some of our vendored code. src/third/redux-persist +src/boot/__tests__/replaceRevive-test.js \ No newline at end of file diff --git a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap new file mode 100644 index 00000000000..e0f30c7d643 --- /dev/null +++ b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap @@ -0,0 +1,23 @@ +// exports[`Immutable Nested stringify 1`] = `"{\"data\":[[\"map\",{\"data\":{\"seq\":{\"data\":[1,2,3,4,5,6,7,8],\"__serializedType__\":\"ImmutableSeq\"},\"stack\":{\"data\":[\"a\",\"b\",\"c\"],\"__serializedType__\":\"ImmutableStack\"}},\"__serializedType__\":\"ImmutableOrderedMap\"}],[\"repeat\",{\"data\":{\"_value\":\"hi\",\"size\":100},\"__serializedType__\":\"ImmutableRepeat\"}]],\"__serializedType__\":\"ImmutableSet\"}"`; + +// exports[`Immutable Record stringify 1`] = `"{\"data\":{\"a\":1,\"b\":3},\"__serializedType__\":\"ImmutableRecord\",\"__serializedRef__\":0}"`; + +// exports[`Immutable Stringify list 1`] = `"{\"data\":[1,2,3,4,5,6,7,8,9,10],\"__serializedType__\":\"ImmutableList\"}"`; + +// exports[`Immutable Stringify map 1`] = `"{\"data\":{\"a\":1,\"b\":2,\"c\":3,\"d\":4},\"__serializedType__\":\"ImmutableMap\"}"`; + +// exports[`Immutable Stringify orderedMap 1`] = `"{\"data\":{\"b\":2,\"a\":1,\"c\":3,\"d\":4},\"__serializedType__\":\"ImmutableOrderedMap\"}"`; + +// exports[`Immutable Stringify orderedSet 1`] = `"{\"data\":[10,9,8,7,6,5,4,3,2,1],\"__serializedType__\":\"ImmutableOrderedSet\"}"`; + +// exports[`Immutable Stringify range 1`] = `"{\"data\":{\"_start\":0,\"_end\":7,\"_step\":1,\"size\":7},\"__serializedType__\":\"ImmutableRange\"}"`; + +// exports[`Immutable Stringify repeat 1`] = `"{\"data\":{\"_value\":\"hi\",\"size\":100},\"__serializedType__\":\"ImmutableRepeat\"}"`; + +// exports[`Immutable Stringify seq 1`] = `"{\"data\":[1,2,3,4,5,6,7,8],\"__serializedType__\":\"ImmutableSeq\"}"`; + +// exports[`Immutable Stringify set 1`] = `"{\"data\":[1,2,3,4,5,6,7,8,9,10],\"__serializedType__\":\"ImmutableSet\"}"`; + +// exports[`Immutable Stringify stack 1`] = `"{\"data\":[\"a\",\"b\",\"c\"],\"__serializedType__\":\"ImmutableStack\"}"`; + +// exports[`Immutable Stringify withTypeKey 1`] = `"{\"__serializedType__\":\"Object\",\"data\":{\"a\":1},\"__serializedType__value\":{\"__serializedType__\":\"Object\",\"data\":{\"b\":[2]},\"__serializedType__value\":{\"c\":[3]}}}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js new file mode 100644 index 00000000000..1715f3dacd3 --- /dev/null +++ b/src/boot/__tests__/replaceRevive-test.js @@ -0,0 +1,150 @@ +/* eslint-disable */ + +// Dummy test; test file must contain at least one test. +test('true is true', () => { + expect(true).toBeTrue(); +}) + +// var Immutable = require('immutable'); +// var Serialize = require('../immutable'); +// var serialize = Serialize(Immutable); +// var stringify = serialize.stringify; +// var parse = serialize.parse; + +// var data = { +// map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), +// orderedMap: Immutable.OrderedMap({ b: 2, a: 1, c: 3, d: 4 }), +// list: Immutable.List([1,2,3,4,5,6,7,8,9,10]), +// range: Immutable.Range(0,7), +// repeat: Immutable.Repeat('hi', 100), +// set: Immutable.Set([10,9,8,7,6,5,4,3,2,1]), +// orderedSet: Immutable.OrderedSet([10,9,8,7,6,5,4,3,2,1]), +// seq: Immutable.Seq.of(1,2,3,4,5,6,7,8), +// stack: Immutable.Stack.of('a', 'b', 'c'), +// withTypeKey: { +// a: 1, +// __serializedType__: { +// b: [2], +// __serializedType__: {c: [3]}, +// }, +// } +// }; + +// describe('Immutable', function () { +// var stringified = {}; +// describe('Stringify', function () { +// Object.keys(data).forEach(function (key) { +// it(key, function () { +// stringified[key] = stringify(data[key]); +// expect(stringified[key]).toMatchSnapshot(); +// }); +// }); +// }); + +// describe('Parse', function () { +// Object.keys(data).forEach(function(key) { +// it(key, function() { +// expect(parse(stringified[key])).toEqual(data[key]); +// }); +// }); +// }); + +// describe('Record', function () { +// var ABRecord = Immutable.Record({ a:1, b:2 }); +// var myRecord = new ABRecord({ b:3 }); + +// var serialize = Serialize(Immutable, [ABRecord]); +// var stringify = serialize.stringify; +// var parse = serialize.parse; +// var stringifiedRecord; + +// it('stringify', function() { +// stringifiedRecord = stringify(myRecord); +// expect(stringifiedRecord).toMatchSnapshot(); +// }); + +// it('parse', function() { +// expect(parse(stringifiedRecord)).toEqual(myRecord); +// }); +// }); + +// describe('Nested', function () { +// var ABRecord = Immutable.Record({ +// map: Immutable.OrderedMap({ seq: data.seq, stack: data.stack }), +// repeat: data.repeat +// }); +// var nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); + +// var serialize = Serialize(Immutable, [ABRecord]); +// var stringify = serialize.stringify; +// var parse = serialize.parse; +// var stringifiedNested; + +// it('stringify', function() { +// stringifiedNested = stringify(nestedData); +// expect(stringifiedNested).toMatchSnapshot(); +// }); + +// it('parse', function() { +// expect(parse(stringifiedNested)).toEqual(nestedData); +// }); +// }); +// describe('With references', function() { +// it('serializes and deserializes', function() { +// var sharedValue = []; +// var record = Immutable.Record({ +// prop: sharedValue +// }); + +// var refs = [record]; + +// var obj = Immutable.Map({ +// fst: new record(), +// scnd: new record() +// }); + +// var serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); +// var parsed = JSON.parse(serialized); + +// var fstProp = parsed.data.fst.data.prop; +// var scndProp = parsed.data.scnd.data.prop; + +// expect(fstProp).toEqual(scndProp); +// expect(Array.isArray(obj.get('fst').get('prop'))); +// }); +// }); + +// describe('Custom replacer and reviver functions', function() { +// var customOneRepresentation = 'one'; + +// function customReplacer(key, value, defaultReplacer) { +// if (value === 1) { +// return { data: customOneRepresentation, __serializedType__: 'number' }; +// } +// return defaultReplacer(key, value); +// } + +// function customReviver(key, value, defaultReviver) { +// if (typeof value === 'object' +// && value.__serializedType__ === 'number' +// && value.data === customOneRepresentation) { +// return 1; +// } +// return defaultReviver(key, value); +// } + +// var serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); + +// Object.keys(data).forEach(function(key) { +// var stringified = serializeCustom.stringify(data[key]); +// it(key, function() { +// var deserialized = serializeCustom.parse(stringified); +// expect(deserialized).toEqual(data[key]); +// if (key === 'map' || key === 'orderedMap') { +// deserializedDefault = parse(stringified); +// expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); +// } +// }); +// }); +// }) +// }); From b9674c9d1c7fa216f58bb3e9344abff46a823f5d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 13:46:42 -0500 Subject: [PATCH 03/46] replaceRevive tests [nfc]: Rewire to import remotedev-serialize as we do. This copy of remotedev-serialize's tests is in our project, not in node_modules/remotedev-serialize. So, replace a relative-path import with a reference to the same thing, as we consume it in our project. --- src/boot/__tests__/replaceRevive-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 1715f3dacd3..bfd26e6ff01 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -6,7 +6,7 @@ test('true is true', () => { }) // var Immutable = require('immutable'); -// var Serialize = require('../immutable'); +// var Serialize = require('remotedev-serialize').immutable; // var serialize = Serialize(Immutable); // var stringify = serialize.stringify; // var parse = serialize.parse; From b9d3c913bea04e7fe8504ce8486275f749a28e16 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 13:48:29 -0500 Subject: [PATCH 04/46] replaceRevive tests [nfc]: Stop using unhandled `Seq.of` alias. It seems the remotedev-serialize maintainers were testing with a different version of `immutable` that allowed using `Seq.of` (ours doesn't, apparently). It's an alias for `Seq.Indexed.of` [1]; so, just use that instead. [1] https://devdocs.io/immutable/index#seq.of --- src/boot/__tests__/replaceRevive-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index bfd26e6ff01..f6e2482583f 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -19,7 +19,7 @@ test('true is true', () => { // repeat: Immutable.Repeat('hi', 100), // set: Immutable.Set([10,9,8,7,6,5,4,3,2,1]), // orderedSet: Immutable.OrderedSet([10,9,8,7,6,5,4,3,2,1]), -// seq: Immutable.Seq.of(1,2,3,4,5,6,7,8), +// seq: Immutable.Seq.Indexed.of(1,2,3,4,5,6,7,8), // stack: Immutable.Stack.of('a', 'b', 'c'), // withTypeKey: { // a: 1, From 3e7ed55d7cab0bef2572fb04c36753edfb34f6ad Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:48:47 -0500 Subject: [PATCH 05/46] replaceRevive tests: Declare a variable properly(-ish). We don't use var -- but it's better than nothing! --- src/boot/__tests__/replaceRevive-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index f6e2482583f..808d516d1d2 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -141,7 +141,7 @@ test('true is true', () => { // var deserialized = serializeCustom.parse(stringified); // expect(deserialized).toEqual(data[key]); // if (key === 'map' || key === 'orderedMap') { -// deserializedDefault = parse(stringified); +// var deserializedDefault = parse(stringified); // expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); // } // }); From 666dc2607c3acc8d4ec3a1d1108bac255ee58d10 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:06:48 -0500 Subject: [PATCH 06/46] replaceRevive tests [nfc]: Update snapshots. The new code is obtained by uncommenting the tests in replaceRevive-test.js and running them (then re-commenting everything for this commit; we'll commit the uncommenting soon). I'm not sure why we suddenly have to use `\\` instead of `\` to escape quotes in the JSON. Other than that, I don't see any discrepancies. --- .../replaceRevive-test.js.snap | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap index e0f30c7d643..5300f516ac5 100644 --- a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap @@ -1,23 +1,23 @@ -// exports[`Immutable Nested stringify 1`] = `"{\"data\":[[\"map\",{\"data\":{\"seq\":{\"data\":[1,2,3,4,5,6,7,8],\"__serializedType__\":\"ImmutableSeq\"},\"stack\":{\"data\":[\"a\",\"b\",\"c\"],\"__serializedType__\":\"ImmutableStack\"}},\"__serializedType__\":\"ImmutableOrderedMap\"}],[\"repeat\",{\"data\":{\"_value\":\"hi\",\"size\":100},\"__serializedType__\":\"ImmutableRepeat\"}]],\"__serializedType__\":\"ImmutableSet\"}"`; +// exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\":{\\"seq\\":{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"},\\"stack\\":{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}],[\\"repeat\\",{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}]],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; -// exports[`Immutable Record stringify 1`] = `"{\"data\":{\"a\":1,\"b\":3},\"__serializedType__\":\"ImmutableRecord\",\"__serializedRef__\":0}"`; +// exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; -// exports[`Immutable Stringify list 1`] = `"{\"data\":[1,2,3,4,5,6,7,8,9,10],\"__serializedType__\":\"ImmutableList\"}"`; +// exports[`Immutable Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; -// exports[`Immutable Stringify map 1`] = `"{\"data\":{\"a\":1,\"b\":2,\"c\":3,\"d\":4},\"__serializedType__\":\"ImmutableMap\"}"`; +// exports[`Immutable Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; -// exports[`Immutable Stringify orderedMap 1`] = `"{\"data\":{\"b\":2,\"a\":1,\"c\":3,\"d\":4},\"__serializedType__\":\"ImmutableOrderedMap\"}"`; +// exports[`Immutable Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; -// exports[`Immutable Stringify orderedSet 1`] = `"{\"data\":[10,9,8,7,6,5,4,3,2,1],\"__serializedType__\":\"ImmutableOrderedSet\"}"`; +// exports[`Immutable Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; -// exports[`Immutable Stringify range 1`] = `"{\"data\":{\"_start\":0,\"_end\":7,\"_step\":1,\"size\":7},\"__serializedType__\":\"ImmutableRange\"}"`; +// exports[`Immutable Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; -// exports[`Immutable Stringify repeat 1`] = `"{\"data\":{\"_value\":\"hi\",\"size\":100},\"__serializedType__\":\"ImmutableRepeat\"}"`; +// exports[`Immutable Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; -// exports[`Immutable Stringify seq 1`] = `"{\"data\":[1,2,3,4,5,6,7,8],\"__serializedType__\":\"ImmutableSeq\"}"`; +// exports[`Immutable Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; -// exports[`Immutable Stringify set 1`] = `"{\"data\":[1,2,3,4,5,6,7,8,9,10],\"__serializedType__\":\"ImmutableSet\"}"`; +// exports[`Immutable Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; -// exports[`Immutable Stringify stack 1`] = `"{\"data\":[\"a\",\"b\",\"c\"],\"__serializedType__\":\"ImmutableStack\"}"`; +// exports[`Immutable Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; -// exports[`Immutable Stringify withTypeKey 1`] = `"{\"__serializedType__\":\"Object\",\"data\":{\"a\":1},\"__serializedType__value\":{\"__serializedType__\":\"Object\",\"data\":{\"b\":[2]},\"__serializedType__value\":{\"c\":[3]}}}"`; +// exports[`Immutable Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; From edfc504afb45b7cfa26b5b218ddbb9e3773f44f6 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:29:35 -0500 Subject: [PATCH 07/46] replaceRevive tests [nfc]: Add a line that Jest wants to add. When we run this with actual snapshot tests, Jest will automatically put this line in. --- .../__snapshots__not_yet_used/replaceRevive-test.js.snap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap index 5300f516ac5..36e38dc30f2 100644 --- a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap @@ -1,3 +1,5 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + // exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\":{\\"seq\\":{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"},\\"stack\\":{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}],[\\"repeat\\",{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}]],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; // exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; From 64d25aee5fd7fbd5fd7fb8f5120ee5181357f05e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:18:39 -0500 Subject: [PATCH 08/46] replaceRevive tests: Uncomment. These tests are now active -- but all they're doing is testing the code we're currently importing from remotedev-serialize. As we transfer that code to our own codebase in the next several commits, we'll point our tests at the transferred code so that they actually test code under our project's direct control. --- .../__snapshots__/replaceRevive-test.js.snap | 25 ++ .../replaceRevive-test.js.snap | 25 -- src/boot/__tests__/replaceRevive-test.js | 291 +++++++++--------- 3 files changed, 168 insertions(+), 173 deletions(-) create mode 100644 src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap delete mode 100644 src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap new file mode 100644 index 00000000000..fd268947c64 --- /dev/null +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\":{\\"seq\\":{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"},\\"stack\\":{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}],[\\"repeat\\",{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}]],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; + +exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; + +exports[`Immutable Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; + +exports[`Immutable Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; + +exports[`Immutable Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; + +exports[`Immutable Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; + +exports[`Immutable Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; + +exports[`Immutable Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; + +exports[`Immutable Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; + +exports[`Immutable Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; + +exports[`Immutable Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; + +exports[`Immutable Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; diff --git a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap deleted file mode 100644 index 36e38dc30f2..00000000000 --- a/src/boot/__tests__/__snapshots__not_yet_used/replaceRevive-test.js.snap +++ /dev/null @@ -1,25 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -// exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\":{\\"seq\\":{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"},\\"stack\\":{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}],[\\"repeat\\",{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}]],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; - -// exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; - -// exports[`Immutable Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; - -// exports[`Immutable Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; - -// exports[`Immutable Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; - -// exports[`Immutable Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; - -// exports[`Immutable Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; - -// exports[`Immutable Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; - -// exports[`Immutable Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; - -// exports[`Immutable Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; - -// exports[`Immutable Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; - -// exports[`Immutable Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 808d516d1d2..24ba8e7688f 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,150 +1,145 @@ /* eslint-disable */ -// Dummy test; test file must contain at least one test. -test('true is true', () => { - expect(true).toBeTrue(); -}) - -// var Immutable = require('immutable'); -// var Serialize = require('remotedev-serialize').immutable; -// var serialize = Serialize(Immutable); -// var stringify = serialize.stringify; -// var parse = serialize.parse; - -// var data = { -// map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), -// orderedMap: Immutable.OrderedMap({ b: 2, a: 1, c: 3, d: 4 }), -// list: Immutable.List([1,2,3,4,5,6,7,8,9,10]), -// range: Immutable.Range(0,7), -// repeat: Immutable.Repeat('hi', 100), -// set: Immutable.Set([10,9,8,7,6,5,4,3,2,1]), -// orderedSet: Immutable.OrderedSet([10,9,8,7,6,5,4,3,2,1]), -// seq: Immutable.Seq.Indexed.of(1,2,3,4,5,6,7,8), -// stack: Immutable.Stack.of('a', 'b', 'c'), -// withTypeKey: { -// a: 1, -// __serializedType__: { -// b: [2], -// __serializedType__: {c: [3]}, -// }, -// } -// }; - -// describe('Immutable', function () { -// var stringified = {}; -// describe('Stringify', function () { -// Object.keys(data).forEach(function (key) { -// it(key, function () { -// stringified[key] = stringify(data[key]); -// expect(stringified[key]).toMatchSnapshot(); -// }); -// }); -// }); - -// describe('Parse', function () { -// Object.keys(data).forEach(function(key) { -// it(key, function() { -// expect(parse(stringified[key])).toEqual(data[key]); -// }); -// }); -// }); - -// describe('Record', function () { -// var ABRecord = Immutable.Record({ a:1, b:2 }); -// var myRecord = new ABRecord({ b:3 }); - -// var serialize = Serialize(Immutable, [ABRecord]); -// var stringify = serialize.stringify; -// var parse = serialize.parse; -// var stringifiedRecord; - -// it('stringify', function() { -// stringifiedRecord = stringify(myRecord); -// expect(stringifiedRecord).toMatchSnapshot(); -// }); - -// it('parse', function() { -// expect(parse(stringifiedRecord)).toEqual(myRecord); -// }); -// }); - -// describe('Nested', function () { -// var ABRecord = Immutable.Record({ -// map: Immutable.OrderedMap({ seq: data.seq, stack: data.stack }), -// repeat: data.repeat -// }); -// var nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); - -// var serialize = Serialize(Immutable, [ABRecord]); -// var stringify = serialize.stringify; -// var parse = serialize.parse; -// var stringifiedNested; - -// it('stringify', function() { -// stringifiedNested = stringify(nestedData); -// expect(stringifiedNested).toMatchSnapshot(); -// }); - -// it('parse', function() { -// expect(parse(stringifiedNested)).toEqual(nestedData); -// }); -// }); -// describe('With references', function() { -// it('serializes and deserializes', function() { -// var sharedValue = []; -// var record = Immutable.Record({ -// prop: sharedValue -// }); - -// var refs = [record]; - -// var obj = Immutable.Map({ -// fst: new record(), -// scnd: new record() -// }); - -// var serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); -// var parsed = JSON.parse(serialized); - -// var fstProp = parsed.data.fst.data.prop; -// var scndProp = parsed.data.scnd.data.prop; - -// expect(fstProp).toEqual(scndProp); -// expect(Array.isArray(obj.get('fst').get('prop'))); -// }); -// }); - -// describe('Custom replacer and reviver functions', function() { -// var customOneRepresentation = 'one'; - -// function customReplacer(key, value, defaultReplacer) { -// if (value === 1) { -// return { data: customOneRepresentation, __serializedType__: 'number' }; -// } -// return defaultReplacer(key, value); -// } - -// function customReviver(key, value, defaultReviver) { -// if (typeof value === 'object' -// && value.__serializedType__ === 'number' -// && value.data === customOneRepresentation) { -// return 1; -// } -// return defaultReviver(key, value); -// } - -// var serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); - -// Object.keys(data).forEach(function(key) { -// var stringified = serializeCustom.stringify(data[key]); -// it(key, function() { -// var deserialized = serializeCustom.parse(stringified); -// expect(deserialized).toEqual(data[key]); -// if (key === 'map' || key === 'orderedMap') { -// var deserializedDefault = parse(stringified); -// expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); -// } -// }); -// }); -// }) -// }); +var Immutable = require('immutable'); +var Serialize = require('remotedev-serialize').immutable; +var serialize = Serialize(Immutable); +var stringify = serialize.stringify; +var parse = serialize.parse; + +var data = { + map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), + orderedMap: Immutable.OrderedMap({ b: 2, a: 1, c: 3, d: 4 }), + list: Immutable.List([1,2,3,4,5,6,7,8,9,10]), + range: Immutable.Range(0,7), + repeat: Immutable.Repeat('hi', 100), + set: Immutable.Set([10,9,8,7,6,5,4,3,2,1]), + orderedSet: Immutable.OrderedSet([10,9,8,7,6,5,4,3,2,1]), + seq: Immutable.Seq.Indexed.of(1,2,3,4,5,6,7,8), + stack: Immutable.Stack.of('a', 'b', 'c'), + withTypeKey: { + a: 1, + __serializedType__: { + b: [2], + __serializedType__: {c: [3]}, + }, + } +}; + +describe('Immutable', function () { + var stringified = {}; + describe('Stringify', function () { + Object.keys(data).forEach(function (key) { + it(key, function () { + stringified[key] = stringify(data[key]); + expect(stringified[key]).toMatchSnapshot(); + }); + }); + }); + + describe('Parse', function () { + Object.keys(data).forEach(function(key) { + it(key, function() { + expect(parse(stringified[key])).toEqual(data[key]); + }); + }); + }); + + describe('Record', function () { + var ABRecord = Immutable.Record({ a:1, b:2 }); + var myRecord = new ABRecord({ b:3 }); + + var serialize = Serialize(Immutable, [ABRecord]); + var stringify = serialize.stringify; + var parse = serialize.parse; + var stringifiedRecord; + + it('stringify', function() { + stringifiedRecord = stringify(myRecord); + expect(stringifiedRecord).toMatchSnapshot(); + }); + + it('parse', function() { + expect(parse(stringifiedRecord)).toEqual(myRecord); + }); + }); + + describe('Nested', function () { + var ABRecord = Immutable.Record({ + map: Immutable.OrderedMap({ seq: data.seq, stack: data.stack }), + repeat: data.repeat + }); + var nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); + + var serialize = Serialize(Immutable, [ABRecord]); + var stringify = serialize.stringify; + var parse = serialize.parse; + var stringifiedNested; + + it('stringify', function() { + stringifiedNested = stringify(nestedData); + expect(stringifiedNested).toMatchSnapshot(); + }); + + it('parse', function() { + expect(parse(stringifiedNested)).toEqual(nestedData); + }); + }); + describe('With references', function() { + it('serializes and deserializes', function() { + var sharedValue = []; + var record = Immutable.Record({ + prop: sharedValue + }); + + var refs = [record]; + + var obj = Immutable.Map({ + fst: new record(), + scnd: new record() + }); + + var serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); + var parsed = JSON.parse(serialized); + + var fstProp = parsed.data.fst.data.prop; + var scndProp = parsed.data.scnd.data.prop; + + expect(fstProp).toEqual(scndProp); + expect(Array.isArray(obj.get('fst').get('prop'))); + }); + }); + + describe('Custom replacer and reviver functions', function() { + var customOneRepresentation = 'one'; + + function customReplacer(key, value, defaultReplacer) { + if (value === 1) { + return { data: customOneRepresentation, __serializedType__: 'number' }; + } + return defaultReplacer(key, value); + } + + function customReviver(key, value, defaultReviver) { + if (typeof value === 'object' + && value.__serializedType__ === 'number' + && value.data === customOneRepresentation) { + return 1; + } + return defaultReviver(key, value); + } + + var serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); + + Object.keys(data).forEach(function(key) { + var stringified = serializeCustom.stringify(data[key]); + it(key, function() { + var deserialized = serializeCustom.parse(stringified); + expect(deserialized).toEqual(data[key]); + if (key === 'map' || key === 'orderedMap') { + var deserializedDefault = parse(stringified); + expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); + } + }); + }); + }) +}); From 69834ac34b96dd64658af3bc36ce4d9131cb2eb4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:46:41 -0500 Subject: [PATCH 09/46] replaceRevive tests [nfc]: Bring closer to our style. Remove the `.prettierignore` entry and the universal `eslint-disable`, reformat the file, and add in some `eslint-disable`s for specific rules. --- .prettierignore | 3 +- src/boot/__tests__/replaceRevive-test.js | 138 ++++++++++++----------- 2 files changed, 74 insertions(+), 67 deletions(-) diff --git a/.prettierignore b/.prettierignore index 4f2feccbe55..1bac849192e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -2,5 +2,4 @@ src/emoji/__tests__/data-test.js # We're not allowing Prettier to touch some of our vendored code. -src/third/redux-persist -src/boot/__tests__/replaceRevive-test.js \ No newline at end of file +src/third/redux-persist \ No newline at end of file diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 24ba8e7688f..1f09b2bdc09 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,116 +1,122 @@ -/* eslint-disable */ +/* eslint-disable no-shadow */ +/* eslint-disable id-match */ +/* eslint-disable no-underscore-dangle */ +/* eslint-disable jest/valid-expect */ +/* eslint-disable jest/no-conditional-expect */ +/* eslint-disable new-cap */ -var Immutable = require('immutable'); -var Serialize = require('remotedev-serialize').immutable; -var serialize = Serialize(Immutable); -var stringify = serialize.stringify; -var parse = serialize.parse; +import Immutable from 'immutable'; +import { immutable as Serialize } from 'remotedev-serialize'; -var data = { +const serialize = Serialize(Immutable); +const stringify = serialize.stringify; +const parse = serialize.parse; + +const data = { map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), orderedMap: Immutable.OrderedMap({ b: 2, a: 1, c: 3, d: 4 }), - list: Immutable.List([1,2,3,4,5,6,7,8,9,10]), - range: Immutable.Range(0,7), + list: Immutable.List([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), + range: Immutable.Range(0, 7), repeat: Immutable.Repeat('hi', 100), - set: Immutable.Set([10,9,8,7,6,5,4,3,2,1]), - orderedSet: Immutable.OrderedSet([10,9,8,7,6,5,4,3,2,1]), - seq: Immutable.Seq.Indexed.of(1,2,3,4,5,6,7,8), + set: Immutable.Set([10, 9, 8, 7, 6, 5, 4, 3, 2, 1]), + orderedSet: Immutable.OrderedSet([10, 9, 8, 7, 6, 5, 4, 3, 2, 1]), + seq: Immutable.Seq.Indexed.of(1, 2, 3, 4, 5, 6, 7, 8), stack: Immutable.Stack.of('a', 'b', 'c'), withTypeKey: { a: 1, __serializedType__: { b: [2], - __serializedType__: {c: [3]}, + __serializedType__: { c: [3] }, }, - } + }, }; -describe('Immutable', function () { - var stringified = {}; - describe('Stringify', function () { - Object.keys(data).forEach(function (key) { - it(key, function () { +describe('Immutable', () => { + const stringified = {}; + describe('Stringify', () => { + Object.keys(data).forEach(key => { + it(key, () => { stringified[key] = stringify(data[key]); expect(stringified[key]).toMatchSnapshot(); }); }); }); - describe('Parse', function () { - Object.keys(data).forEach(function(key) { - it(key, function() { + describe('Parse', () => { + Object.keys(data).forEach(key => { + it(key, () => { expect(parse(stringified[key])).toEqual(data[key]); }); }); }); - describe('Record', function () { - var ABRecord = Immutable.Record({ a:1, b:2 }); - var myRecord = new ABRecord({ b:3 }); + describe('Record', () => { + const ABRecord = Immutable.Record({ a: 1, b: 2 }); + const myRecord = new ABRecord({ b: 3 }); - var serialize = Serialize(Immutable, [ABRecord]); - var stringify = serialize.stringify; - var parse = serialize.parse; - var stringifiedRecord; + const serialize = Serialize(Immutable, [ABRecord]); + const stringify = serialize.stringify; + const parse = serialize.parse; + let stringifiedRecord; - it('stringify', function() { + it('stringify', () => { stringifiedRecord = stringify(myRecord); expect(stringifiedRecord).toMatchSnapshot(); }); - it('parse', function() { + it('parse', () => { expect(parse(stringifiedRecord)).toEqual(myRecord); }); }); - describe('Nested', function () { - var ABRecord = Immutable.Record({ + describe('Nested', () => { + const ABRecord = Immutable.Record({ map: Immutable.OrderedMap({ seq: data.seq, stack: data.stack }), - repeat: data.repeat + repeat: data.repeat, }); - var nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); + const nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); - var serialize = Serialize(Immutable, [ABRecord]); - var stringify = serialize.stringify; - var parse = serialize.parse; - var stringifiedNested; + const serialize = Serialize(Immutable, [ABRecord]); + const stringify = serialize.stringify; + const parse = serialize.parse; + let stringifiedNested; - it('stringify', function() { + it('stringify', () => { stringifiedNested = stringify(nestedData); expect(stringifiedNested).toMatchSnapshot(); }); - it('parse', function() { + it('parse', () => { expect(parse(stringifiedNested)).toEqual(nestedData); }); }); - describe('With references', function() { - it('serializes and deserializes', function() { - var sharedValue = []; - var record = Immutable.Record({ - prop: sharedValue + describe('With references', () => { + it('serializes and deserializes', () => { + const sharedValue = []; + const record = Immutable.Record({ + prop: sharedValue, }); - var refs = [record]; + const refs = [record]; - var obj = Immutable.Map({ + const obj = Immutable.Map({ fst: new record(), - scnd: new record() + scnd: new record(), }); - var serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); - var parsed = JSON.parse(serialized); + const serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); + const parsed = JSON.parse(serialized); - var fstProp = parsed.data.fst.data.prop; - var scndProp = parsed.data.scnd.data.prop; + const fstProp = parsed.data.fst.data.prop; + const scndProp = parsed.data.scnd.data.prop; expect(fstProp).toEqual(scndProp); expect(Array.isArray(obj.get('fst').get('prop'))); }); }); - describe('Custom replacer and reviver functions', function() { - var customOneRepresentation = 'one'; + describe('Custom replacer and reviver functions', () => { + const customOneRepresentation = 'one'; function customReplacer(key, value, defaultReplacer) { if (value === 1) { @@ -120,26 +126,28 @@ describe('Immutable', function () { } function customReviver(key, value, defaultReviver) { - if (typeof value === 'object' - && value.__serializedType__ === 'number' - && value.data === customOneRepresentation) { + if ( + typeof value === 'object' + && value.__serializedType__ === 'number' + && value.data === customOneRepresentation + ) { return 1; } return defaultReviver(key, value); } - var serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); + const serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); - Object.keys(data).forEach(function(key) { - var stringified = serializeCustom.stringify(data[key]); - it(key, function() { - var deserialized = serializeCustom.parse(stringified); + Object.keys(data).forEach(key => { + const stringified = serializeCustom.stringify(data[key]); + it(key, () => { + const deserialized = serializeCustom.parse(stringified); expect(deserialized).toEqual(data[key]); if (key === 'map' || key === 'orderedMap') { - var deserializedDefault = parse(stringified); + const deserializedDefault = parse(stringified); expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); } }); }); - }) + }); }); From 84054e7b7326dce919e48f5df2aba107e26d9b1e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:52:30 -0500 Subject: [PATCH 10/46] replaceRevive tests [nfc]: Fix `new-cap` violations. It gets capitalized in the examples in the ImmutableJS docs: https://immutable-js.github.io/immutable-js/docs/#/Record. --- src/boot/__tests__/replaceRevive-test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 1f09b2bdc09..80056dcf036 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -3,7 +3,6 @@ /* eslint-disable no-underscore-dangle */ /* eslint-disable jest/valid-expect */ /* eslint-disable jest/no-conditional-expect */ -/* eslint-disable new-cap */ import Immutable from 'immutable'; import { immutable as Serialize } from 'remotedev-serialize'; @@ -93,15 +92,15 @@ describe('Immutable', () => { describe('With references', () => { it('serializes and deserializes', () => { const sharedValue = []; - const record = Immutable.Record({ + const Record = Immutable.Record({ prop: sharedValue, }); - const refs = [record]; + const refs = [Record]; const obj = Immutable.Map({ - fst: new record(), - scnd: new record(), + fst: new Record(), + scnd: new Record(), }); const serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); From 52da22f1aeceeaeac001343476972bf959c185e2 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 14:58:21 -0500 Subject: [PATCH 11/46] replaceRevive tests [nfc]: Fix `no-shadow` violations. --- .../__snapshots__/replaceRevive-test.js.snap | 20 ++++----- src/boot/__tests__/replaceRevive-test.js | 45 ++++++++++--------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index fd268947c64..d68d63c586e 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -4,22 +4,22 @@ exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\": exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; -exports[`Immutable Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; +exports[`Immutable Stringify / Parse Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; -exports[`Immutable Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; +exports[`Immutable Stringify / Parse Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; -exports[`Immutable Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; +exports[`Immutable Stringify / Parse Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; -exports[`Immutable Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; +exports[`Immutable Stringify / Parse Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; -exports[`Immutable Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; +exports[`Immutable Stringify / Parse Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; -exports[`Immutable Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; +exports[`Immutable Stringify / Parse Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; -exports[`Immutable Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; +exports[`Immutable Stringify / Parse Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; -exports[`Immutable Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; +exports[`Immutable Stringify / Parse Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; -exports[`Immutable Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; +exports[`Immutable Stringify / Parse Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; -exports[`Immutable Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; +exports[`Immutable Stringify / Parse Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 80056dcf036..f13822aff0d 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,4 +1,3 @@ -/* eslint-disable no-shadow */ /* eslint-disable id-match */ /* eslint-disable no-underscore-dangle */ /* eslint-disable jest/valid-expect */ @@ -31,20 +30,22 @@ const data = { }; describe('Immutable', () => { - const stringified = {}; - describe('Stringify', () => { - Object.keys(data).forEach(key => { - it(key, () => { - stringified[key] = stringify(data[key]); - expect(stringified[key]).toMatchSnapshot(); + describe('Stringify / Parse', () => { + const stringified = {}; + describe('Stringify', () => { + Object.keys(data).forEach(key => { + it(key, () => { + stringified[key] = stringify(data[key]); + expect(stringified[key]).toMatchSnapshot(); + }); }); }); - }); - describe('Parse', () => { - Object.keys(data).forEach(key => { - it(key, () => { - expect(parse(stringified[key])).toEqual(data[key]); + describe('Parse', () => { + Object.keys(data).forEach(key => { + it(key, () => { + expect(parse(stringified[key])).toEqual(data[key]); + }); }); }); }); @@ -53,18 +54,18 @@ describe('Immutable', () => { const ABRecord = Immutable.Record({ a: 1, b: 2 }); const myRecord = new ABRecord({ b: 3 }); - const serialize = Serialize(Immutable, [ABRecord]); - const stringify = serialize.stringify; - const parse = serialize.parse; + const serializeWithRefs = Serialize(Immutable, [ABRecord]); + const stringifyWithRefs = serializeWithRefs.stringify; + const parseWithRefs = serializeWithRefs.parse; let stringifiedRecord; it('stringify', () => { - stringifiedRecord = stringify(myRecord); + stringifiedRecord = stringifyWithRefs(myRecord); expect(stringifiedRecord).toMatchSnapshot(); }); it('parse', () => { - expect(parse(stringifiedRecord)).toEqual(myRecord); + expect(parseWithRefs(stringifiedRecord)).toEqual(myRecord); }); }); @@ -75,18 +76,18 @@ describe('Immutable', () => { }); const nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); - const serialize = Serialize(Immutable, [ABRecord]); - const stringify = serialize.stringify; - const parse = serialize.parse; + const serializeWithRefs = Serialize(Immutable, [ABRecord]); + const stringifyWithRefs = serializeWithRefs.stringify; + const parseWithRefs = serializeWithRefs.parse; let stringifiedNested; it('stringify', () => { - stringifiedNested = stringify(nestedData); + stringifiedNested = stringifyWithRefs(nestedData); expect(stringifiedNested).toMatchSnapshot(); }); it('parse', () => { - expect(parse(stringifiedNested)).toEqual(nestedData); + expect(parseWithRefs(stringifiedNested)).toEqual(nestedData); }); }); describe('With references', () => { From a71000ee7dbb617d9378ccc2e350097d0e43fad8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 15:00:54 -0500 Subject: [PATCH 12/46] replaceRevive tests: Fix `valid-expect` violation. Without a matcher, it looks like this `expect` call was doing precisely nothing! Glad to have this `jest/valid-expect` rule. --- src/boot/__tests__/replaceRevive-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index f13822aff0d..ac06118a228 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,6 +1,5 @@ /* eslint-disable id-match */ /* eslint-disable no-underscore-dangle */ -/* eslint-disable jest/valid-expect */ /* eslint-disable jest/no-conditional-expect */ import Immutable from 'immutable'; @@ -111,7 +110,7 @@ describe('Immutable', () => { const scndProp = parsed.data.scnd.data.prop; expect(fstProp).toEqual(scndProp); - expect(Array.isArray(obj.get('fst').get('prop'))); + expect(Array.isArray(obj.get('fst').get('prop'))).toBeTrue(); }); }); From d5a9357c0f624da36819a38ce5c54704c8bdbdc1 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 15:10:11 -0500 Subject: [PATCH 13/46] replaceRevive tests [nfc]: Move jest/no-conditional-expect, add comments. --- src/boot/__tests__/replaceRevive-test.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index ac06118a228..7b771735ed0 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,6 +1,5 @@ /* eslint-disable id-match */ /* eslint-disable no-underscore-dangle */ -/* eslint-disable jest/no-conditional-expect */ import Immutable from 'immutable'; import { immutable as Serialize } from 'remotedev-serialize'; @@ -141,9 +140,21 @@ describe('Immutable', () => { const stringified = serializeCustom.stringify(data[key]); it(key, () => { const deserialized = serializeCustom.parse(stringified); + // Make sure serializeCustom round-trips expect(deserialized).toEqual(data[key]); if (key === 'map' || key === 'orderedMap') { const deserializedDefault = parse(stringified); + // Make sure we actually stored `1` in the customReplacer's + // representation, 'one'. + // + // If we were going to keep this test, it would probably be + // wise to listen to `jest/no-conditional-expect` (see + // https://github.com/zulip/zulip-mobile/pull/4348#discussion_r548761362) + // -- but we're not; we'll remove this entire `describe` + // block soon, when we're no longer using the + // `Serialize.immutable` factory. + // + // eslint-disable-next-line jest/no-conditional-expect expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); } }); From ad1ca60a706871e285a78cbfdff07384f56fd2ff Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 15:20:48 -0500 Subject: [PATCH 14/46] replaceRevive tests: Remove all tests that used refs. `refs` is a param of `Serialize.immutable`, and we've never passed an interesting value for it. Right away, when we start inlining remotedev-serialize, we'll solidify that with a design that assumes we'll never pass an interesting value for it. --- .../__snapshots__/replaceRevive-test.js.snap | 4 -- src/boot/__tests__/replaceRevive-test.js | 65 ------------------- 2 files changed, 69 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index d68d63c586e..92edb8de359 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -1,9 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Immutable Nested stringify 1`] = `"{\\"data\\":[[\\"map\\",{\\"data\\":{\\"seq\\":{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"},\\"stack\\":{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}],[\\"repeat\\",{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}]],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; - -exports[`Immutable Record stringify 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":3},\\"__serializedType__\\":\\"ImmutableRecord\\",\\"__serializedRef__\\":0}"`; - exports[`Immutable Stringify / Parse Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; exports[`Immutable Stringify / Parse Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 7b771735ed0..8d0f42f86a0 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -48,71 +48,6 @@ describe('Immutable', () => { }); }); - describe('Record', () => { - const ABRecord = Immutable.Record({ a: 1, b: 2 }); - const myRecord = new ABRecord({ b: 3 }); - - const serializeWithRefs = Serialize(Immutable, [ABRecord]); - const stringifyWithRefs = serializeWithRefs.stringify; - const parseWithRefs = serializeWithRefs.parse; - let stringifiedRecord; - - it('stringify', () => { - stringifiedRecord = stringifyWithRefs(myRecord); - expect(stringifiedRecord).toMatchSnapshot(); - }); - - it('parse', () => { - expect(parseWithRefs(stringifiedRecord)).toEqual(myRecord); - }); - }); - - describe('Nested', () => { - const ABRecord = Immutable.Record({ - map: Immutable.OrderedMap({ seq: data.seq, stack: data.stack }), - repeat: data.repeat, - }); - const nestedData = Immutable.Set(ABRecord(), data.orderedSet, data.range); - - const serializeWithRefs = Serialize(Immutable, [ABRecord]); - const stringifyWithRefs = serializeWithRefs.stringify; - const parseWithRefs = serializeWithRefs.parse; - let stringifiedNested; - - it('stringify', () => { - stringifiedNested = stringifyWithRefs(nestedData); - expect(stringifiedNested).toMatchSnapshot(); - }); - - it('parse', () => { - expect(parseWithRefs(stringifiedNested)).toEqual(nestedData); - }); - }); - describe('With references', () => { - it('serializes and deserializes', () => { - const sharedValue = []; - const Record = Immutable.Record({ - prop: sharedValue, - }); - - const refs = [Record]; - - const obj = Immutable.Map({ - fst: new Record(), - scnd: new Record(), - }); - - const serialized = stringify(obj, Serialize(Immutable, refs).replacer, null, true); - const parsed = JSON.parse(serialized); - - const fstProp = parsed.data.fst.data.prop; - const scndProp = parsed.data.scnd.data.prop; - - expect(fstProp).toEqual(scndProp); - expect(Array.isArray(obj.get('fst').get('prop'))).toBeTrue(); - }); - }); - describe('Custom replacer and reviver functions', () => { const customOneRepresentation = 'one'; From 5130b74948050fbd45ab641fbf9ea2b6826adf77 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 11:21:29 -0500 Subject: [PATCH 15/46] store [nfc]: Inline remotedev-serialize (1/x). It turns out that remotedev-serialize is doing some tasks that we don't need it to do. See discussion for more background [1]. The changes we'd like to make to fix that will reduce the amount of code used to implement our replace/revive logic, and it'll make sense to just fold remotedev-serialize into our codebase instead of depending on it as a separate module. One particular thing we want to do is cut `jsan`, something used by remotedev-serialize, out of the picture. One main feature `jsan` announces is that it can handle serializing an object with circular references -- but we clearly don't need this, since our data is naturally free of them. And as it checks for circular references, it makes a deep copy of the whole data structure. We also don't need any of the other fancy features `jsan` advertises, like persisting functions. [2] So we can potentially gain a good performance boost by just cutting `jsan` out of the picture. We'll do that after all these "Inline remotedev-serialize (n/x)" commits. Another thing (and I opened this as zulip/remotedev-serialize#2 before we decided to inline remotedev-serialize), we've observed [3] that we can save maybe >20% of the time we periodically spend persisting the state by removing various lines in remotedev-serialize itself (not `jsan`) that handle Immutable.Map data structures that we don't currently use. Start by cutting out the `Serialize.immutable` factory and just writing out `stringify` and `parse` (previously obtained by calling `Serialize.immutable`) ourselves, based on `Serialize.immutable`'s implementation and the inputs we'd been giving it. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.60remotedev-serialize.60.20and.20.60jsan.60/near/1082744 [2] It turns out that the circular reference-handling code does helpfully recurse through and apply our custom replacer function on values before they've been transformed by their `toJSON` methods. We'll handle that wrinkle when we take `jsan` out, a few commits from now. [3] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/user_avatar_url_field_optional/near/1082087 --- package.json | 1 + src/boot/store.js | 33 +++++++++++++++++++++++++++++++-- yarn.lock | 2 +- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 096d6d3c60b..f578997ae78 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "expo-splash-screen": "^0.5.0", "immutable": "^4.0.0-rc.12", "invariant": "^2.2.4", + "jsan": "3.1.13", "json-stringify-safe": "^5.0.1", "katex": "^0.11.1", "lodash.escape": "^4.0.1", diff --git a/src/boot/store.js b/src/boot/store.js index c944a886ebc..9971dd18a23 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -1,11 +1,21 @@ /* @flow strict-local */ + +// Switch off some rules for this file as we continue folding +// remotedev-serialize into our code; we'll remove these soon. +// +/* eslint-disable id-match */ +/* eslint-disable func-names */ +/* flowlint untyped-import:off */ + import { applyMiddleware, compose, createStore } from 'redux'; import type { Store } from 'redux'; import thunkMiddleware from 'redux-thunk'; import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; -import * as Serialize from 'remotedev-serialize'; +import jsan from 'jsan'; +import serialize from 'remotedev-serialize/immutable/serialize'; +import options from 'remotedev-serialize/constants/options'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -377,7 +387,26 @@ const customReviver = (key, value, defaultReviver) => { return defaultReviver(key, value); }; -const { stringify, parse } = Serialize.immutable(Immutable, null, customReplacer, customReviver); +/** PRIVATE: Exported only for tests. */ +// Recently inlined from +// node_modules/remotedev-serialize/immutable/index.js; this will +// change over the next few commits. +export const stringify = function (data: mixed): string { + return jsan.stringify( + data, + serialize(Immutable, null, customReplacer, customReviver).replacer, + null, + options, + ); +}; + +/** PRIVATE: Exported only for tests. */ +// Recently inlined from +// node_modules/remotedev-serialize/immutable/index.js; this will +// change over the next few commits. +export const parse = function (data: string): mixed { + return jsan.parse(data, serialize(Immutable, null, customReplacer, customReviver).reviver); +}; /** * The config options to pass to redux-persist. diff --git a/yarn.lock b/yarn.lock index 6355be3d68d..dfd16cf018b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7669,7 +7669,7 @@ js-yaml@^3.13.0, js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -jsan@^3.1.13: +jsan@3.1.13, jsan@^3.1.13: version "3.1.13" resolved "https://registry.yarnpkg.com/jsan/-/jsan-3.1.13.tgz#4de8c7bf8d1cfcd020c313d438f930cec4b91d86" integrity sha512-9kGpCsGHifmw6oJet+y8HaCl14y7qgAsxVdV3pCHDySNR3BfDC30zgkssd7x5LRVAT22dnpbe9JdzzmXZnq9/g== From 0c8903213097f5c6db95814d6a2128d85b3b229f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 22 Dec 2020 15:52:57 -0500 Subject: [PATCH 16/46] replaceRevive tests: Start testing exports of store.js. --- src/boot/__tests__/replaceRevive-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 8d0f42f86a0..4acf3f7bc7a 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -4,9 +4,7 @@ import Immutable from 'immutable'; import { immutable as Serialize } from 'remotedev-serialize'; -const serialize = Serialize(Immutable); -const stringify = serialize.stringify; -const parse = serialize.parse; +import { stringify, parse } from '../store'; const data = { map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), From cc1398e146f1e8fbd6e61d4061b128f4c3844134 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 13:31:26 -0500 Subject: [PATCH 17/46] replaceRevive tests: Test `stringify` and `parse` with our own data types. In a recent commit, we stopped using remotedev-serialize's `Serialize.immutable` factory to generate `stringify` and `parse` based on a passed `customReplacer` and `customReviver`, instead hard-coding `stringify` and `parse` in our own code using our custom replacer and reviver logic. Since we don't plan to use `Serialize.immutable` again, we don't need tests to confirm that it does the right think with a provided `customReplacer` and `customReplacer` -- so, delete those. But we should ensure that our custom data types are tested just as thorougly as the `Immutable.*` ones. So, add those to `data`. --- .../__snapshots__/replaceRevive-test.js.snap | 10 +++ src/boot/__tests__/replaceRevive-test.js | 64 +++++-------------- 2 files changed, 25 insertions(+), 49 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index 92edb8de359..1adad935b7b 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -1,5 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Immutable Stringify / Parse Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/avatar/1\\",\\"__serializedType__\\":\\"FallbackAvatarURL\\"}"`; + +exports[`Immutable Stringify / Parse Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`; + exports[`Immutable Stringify / Parse Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; exports[`Immutable Stringify / Parse Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; @@ -18,4 +22,10 @@ exports[`Immutable Stringify / Parse Stringify set 1`] = `"{\\"data\\":[1,2,3,4, exports[`Immutable Stringify / Parse Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; +exports[`Immutable Stringify / Parse Stringify uploadedAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2\\",\\"__serializedType__\\":\\"UploadedAvatarURL\\"}"`; + +exports[`Immutable Stringify / Parse Stringify url 1`] = `"{\\"data\\":\\"https://chat.zulip.org/\\",\\"__serializedType__\\":\\"URL\\"}"`; + exports[`Immutable Stringify / Parse Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; + +exports[`Immutable Stringify / Parse Stringify zulipVersion 1`] = `"{\\"data\\":\\"3.0.0\\",\\"__serializedType__\\":\\"ZulipVersion\\"}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 4acf3f7bc7a..ecebf8f6a76 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -2,9 +2,11 @@ /* eslint-disable no-underscore-dangle */ import Immutable from 'immutable'; -import { immutable as Serialize } from 'remotedev-serialize'; +import { FallbackAvatarURL, GravatarURL, UploadedAvatarURL } from '../../utils/avatar'; +import { ZulipVersion } from '../../utils/zulipVersion'; import { stringify, parse } from '../store'; +import * as eg from '../../__tests__/lib/exampleData'; const data = { map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), @@ -16,6 +18,18 @@ const data = { orderedSet: Immutable.OrderedSet([10, 9, 8, 7, 6, 5, 4, 3, 2, 1]), seq: Immutable.Seq.Indexed.of(1, 2, 3, 4, 5, 6, 7, 8), stack: Immutable.Stack.of('a', 'b', 'c'), + zulipVersion: new ZulipVersion('3.0.0'), + url: new URL('https://chat.zulip.org'), + gravatarURL: GravatarURL.validateAndConstructInstance({ email: eg.selfUser.email }), + uploadedAvatarURL: UploadedAvatarURL.validateAndConstructInstance({ + realm: eg.realm, + absoluteOrRelativeUrl: + '/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2', + }), + fallbackAvatarURL: FallbackAvatarURL.validateAndConstructInstance({ + realm: eg.realm, + userId: 1, + }), withTypeKey: { a: 1, __serializedType__: { @@ -45,52 +59,4 @@ describe('Immutable', () => { }); }); }); - - describe('Custom replacer and reviver functions', () => { - const customOneRepresentation = 'one'; - - function customReplacer(key, value, defaultReplacer) { - if (value === 1) { - return { data: customOneRepresentation, __serializedType__: 'number' }; - } - return defaultReplacer(key, value); - } - - function customReviver(key, value, defaultReviver) { - if ( - typeof value === 'object' - && value.__serializedType__ === 'number' - && value.data === customOneRepresentation - ) { - return 1; - } - return defaultReviver(key, value); - } - - const serializeCustom = Serialize(Immutable, null, customReplacer, customReviver); - - Object.keys(data).forEach(key => { - const stringified = serializeCustom.stringify(data[key]); - it(key, () => { - const deserialized = serializeCustom.parse(stringified); - // Make sure serializeCustom round-trips - expect(deserialized).toEqual(data[key]); - if (key === 'map' || key === 'orderedMap') { - const deserializedDefault = parse(stringified); - // Make sure we actually stored `1` in the customReplacer's - // representation, 'one'. - // - // If we were going to keep this test, it would probably be - // wise to listen to `jest/no-conditional-expect` (see - // https://github.com/zulip/zulip-mobile/pull/4348#discussion_r548761362) - // -- but we're not; we'll remove this entire `describe` - // block soon, when we're no longer using the - // `Serialize.immutable` factory. - // - // eslint-disable-next-line jest/no-conditional-expect - expect(deserializedDefault.get('a')).toEqual(customOneRepresentation); - } - }); - }); - }); }); From 53038b6f9114bf48e9ee9d4f3115b8fcc9054842 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 14:19:58 -0500 Subject: [PATCH 18/46] replaceRevive tests [nfc]: Start type-checking. The previous commit removed some now-unnecessary code that would have been a bit annoying to type-check with Flow. --- src/boot/__tests__/replaceRevive-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index ecebf8f6a76..2d87e4bdf5d 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -1,3 +1,4 @@ +/* @flow strict-local */ /* eslint-disable id-match */ /* eslint-disable no-underscore-dangle */ From cc43d85a8993700e4e332e90b2e9a194357f48d4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 13:41:55 -0500 Subject: [PATCH 19/46] replaceRevive tests [nfc]: Remove some unnecessary levels of nesting. --- .../__snapshots__/replaceRevive-test.js.snap | 30 +++++++++---------- src/boot/__tests__/replaceRevive-test.js | 28 ++++++++--------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index 1adad935b7b..fa4cfeb0f12 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -1,31 +1,31 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Immutable Stringify / Parse Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/avatar/1\\",\\"__serializedType__\\":\\"FallbackAvatarURL\\"}"`; +exports[`Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/avatar/1\\",\\"__serializedType__\\":\\"FallbackAvatarURL\\"}"`; -exports[`Immutable Stringify / Parse Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`; +exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`; -exports[`Immutable Stringify / Parse Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; +exports[`Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; -exports[`Immutable Stringify / Parse Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; +exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; -exports[`Immutable Stringify / Parse Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; +exports[`Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; -exports[`Immutable Stringify / Parse Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; +exports[`Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; -exports[`Immutable Stringify / Parse Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; +exports[`Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; -exports[`Immutable Stringify / Parse Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; +exports[`Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; -exports[`Immutable Stringify / Parse Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; +exports[`Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; -exports[`Immutable Stringify / Parse Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; +exports[`Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; -exports[`Immutable Stringify / Parse Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; +exports[`Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; -exports[`Immutable Stringify / Parse Stringify uploadedAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2\\",\\"__serializedType__\\":\\"UploadedAvatarURL\\"}"`; +exports[`Stringify uploadedAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2\\",\\"__serializedType__\\":\\"UploadedAvatarURL\\"}"`; -exports[`Immutable Stringify / Parse Stringify url 1`] = `"{\\"data\\":\\"https://chat.zulip.org/\\",\\"__serializedType__\\":\\"URL\\"}"`; +exports[`Stringify url 1`] = `"{\\"data\\":\\"https://chat.zulip.org/\\",\\"__serializedType__\\":\\"URL\\"}"`; -exports[`Immutable Stringify / Parse Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; +exports[`Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; -exports[`Immutable Stringify / Parse Stringify zulipVersion 1`] = `"{\\"data\\":\\"3.0.0\\",\\"__serializedType__\\":\\"ZulipVersion\\"}"`; +exports[`Stringify zulipVersion 1`] = `"{\\"data\\":\\"3.0.0\\",\\"__serializedType__\\":\\"ZulipVersion\\"}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 2d87e4bdf5d..d26f49a9a79 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -40,24 +40,20 @@ const data = { }, }; -describe('Immutable', () => { - describe('Stringify / Parse', () => { - const stringified = {}; - describe('Stringify', () => { - Object.keys(data).forEach(key => { - it(key, () => { - stringified[key] = stringify(data[key]); - expect(stringified[key]).toMatchSnapshot(); - }); - }); +const stringified = {}; +describe('Stringify', () => { + Object.keys(data).forEach(key => { + it(key, () => { + stringified[key] = stringify(data[key]); + expect(stringified[key]).toMatchSnapshot(); }); + }); +}); - describe('Parse', () => { - Object.keys(data).forEach(key => { - it(key, () => { - expect(parse(stringified[key])).toEqual(data[key]); - }); - }); +describe('Parse', () => { + Object.keys(data).forEach(key => { + it(key, () => { + expect(parse(stringified[key])).toEqual(data[key]); }); }); }); From 2d617f6870824ed0b9e133c0404c65c1770e3468 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 11:45:28 -0500 Subject: [PATCH 20/46] store [nfc]: Inline remotedev-serialize (2/x). --- src/boot/store.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 9971dd18a23..3773d4e682f 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -15,7 +15,6 @@ import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; import jsan from 'jsan'; import serialize from 'remotedev-serialize/immutable/serialize'; -import options from 'remotedev-serialize/constants/options'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -345,6 +344,23 @@ provideLoggingContext(() => ({ */ const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; +// Recently inlined from +// node_modules/remotedev-serialize/constants/options.js; this will +// change over the next few commits. +const jsanOptions = { + refs: false, // references can't be resolved on the original Immutable structure + date: true, + function: true, + regex: true, + undefined: true, + error: true, + symbol: true, + map: true, + set: true, + nan: true, + infinity: true, +}; + const customReplacer = (key, value, defaultReplacer) => { if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; @@ -396,7 +412,7 @@ export const stringify = function (data: mixed): string { data, serialize(Immutable, null, customReplacer, customReviver).replacer, null, - options, + jsanOptions, ); }; From d4fcb1cd7d0ff9b63497c7c70699501e1df0e603 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:16:26 -0500 Subject: [PATCH 21/46] store [nfc]: Inline remotedev-serialize (3/x). Bring in the `serialize` factory. We'll follow this with a non-NFC commit to trim out several of the `if` cases we don't need, saving some computation time and shortening this quite a lot. --- src/boot/store.js | 100 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) diff --git a/src/boot/store.js b/src/boot/store.js index 3773d4e682f..6935603cf3c 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -3,6 +3,7 @@ // Switch off some rules for this file as we continue folding // remotedev-serialize into our code; we'll remove these soon. // +/* eslint-disable no-underscore-dangle */ /* eslint-disable id-match */ /* eslint-disable func-names */ /* flowlint untyped-import:off */ @@ -14,7 +15,7 @@ import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; import jsan from 'jsan'; -import serialize from 'remotedev-serialize/immutable/serialize'; +import { mark, extract, refer } from 'remotedev-serialize/helpers'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -403,6 +404,103 @@ const customReviver = (key, value, defaultReviver) => { return defaultReviver(key, value); }; +// Recently inlined from +// node_modules/remotedev-serialize/immutable/serialize.js; this will +// change over the next few commits. +const serialize = function serialize(_Immutable, refs, _customReplacer, _customReviver) { + function replacer(key, value) { + if (value instanceof _Immutable.Record) { + return refer(value, 'ImmutableRecord', 'toObject', refs); + } + if (value instanceof _Immutable.Range) { + return extract(value, 'ImmutableRange'); + } + if (value instanceof _Immutable.Repeat) { + return extract(value, 'ImmutableRepeat'); + } + if (_Immutable.OrderedMap.isOrderedMap(value)) { + return mark(value, 'ImmutableOrderedMap', 'toObject'); + } + if (_Immutable.Map.isMap(value)) { + return mark(value, 'ImmutableMap', 'toObject'); + } + if (_Immutable.List.isList(value)) { + return mark(value, 'ImmutableList', 'toArray'); + } + if (_Immutable.OrderedSet.isOrderedSet(value)) { + return mark(value, 'ImmutableOrderedSet', 'toArray'); + } + if (_Immutable.Set.isSet(value)) { + return mark(value, 'ImmutableSet', 'toArray'); + } + if (_Immutable.Seq.isSeq(value)) { + return mark(value, 'ImmutableSeq', 'toArray'); + } + if (_Immutable.Stack.isStack(value)) { + return mark(value, 'ImmutableStack', 'toArray'); + } + if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + const copy = { ...value }; + delete copy.__serializedType__; + return { + __serializedType__: 'Object', + data: copy, + __serializedType__value: value.__serializedType__, + }; + } + return value; + } + + function reviver(key, value) { + if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + const data = value.data; + switch (value.__serializedType__) { + case 'ImmutableMap': + return _Immutable.Map(data); + case 'ImmutableOrderedMap': + return _Immutable.OrderedMap(data); + case 'ImmutableList': + return _Immutable.List(data); + case 'ImmutableRange': + return _Immutable.Range(data._start, data._end, data._step); + case 'ImmutableRepeat': + return _Immutable.Repeat(data._value, data.size); + case 'ImmutableSet': + return _Immutable.Set(data); + case 'ImmutableOrderedSet': + return _Immutable.OrderedSet(data); + case 'ImmutableSeq': + return _Immutable.Seq(data); + case 'ImmutableStack': + return _Immutable.Stack(data); + case 'ImmutableRecord': + return refs && refs[value.__serializedRef__] + ? new refs[value.__serializedRef__](data) + : _Immutable.Map(data); + case 'Object': + return { ...data, __serializedType__: value.__serializedType__value }; + default: + return data; + } + } + return value; + } + + return { + replacer: _customReplacer + ? function (key, value) { + return _customReplacer(key, value, replacer); + } + : replacer, + reviver: _customReviver + ? function (key, value) { + return _customReviver(key, value, reviver); + } + : reviver, + options: jsanOptions, + }; +}; + /** PRIVATE: Exported only for tests. */ // Recently inlined from // node_modules/remotedev-serialize/immutable/index.js; this will From 85c16066bce800d5d36380136421ba5a9aee14b9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:32:37 -0500 Subject: [PATCH 22/46] store: Remove replace/revive handlers for Immutable things we don't use. This has been observed to save >20% of the time it takes to persist our Redux storage to disk [1]. It's also convenient for the mechanics of inlining remotedev-serialize; e.g., we won't have to spell out `refer` at all, since it's not used after this commit. Originally opened as zulip/remotedev-serialize#2, before it became clear that we'd like to inline remotedev-serialize. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/user_avatar_url_field_optional/near/1082087 --- .../__snapshots__/replaceRevive-test.js.snap | 16 ------ src/boot/__tests__/replaceRevive-test.js | 8 --- src/boot/store.js | 49 +------------------ 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index fa4cfeb0f12..0ddb600885d 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -4,24 +4,8 @@ exports[`Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.exampl exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`; -exports[`Stringify list 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableList\\"}"`; - exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; -exports[`Stringify orderedMap 1`] = `"{\\"data\\":{\\"b\\":2,\\"a\\":1,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableOrderedMap\\"}"`; - -exports[`Stringify orderedSet 1`] = `"{\\"data\\":[10,9,8,7,6,5,4,3,2,1],\\"__serializedType__\\":\\"ImmutableOrderedSet\\"}"`; - -exports[`Stringify range 1`] = `"{\\"data\\":{\\"_start\\":0,\\"_end\\":7,\\"_step\\":1,\\"size\\":7},\\"__serializedType__\\":\\"ImmutableRange\\"}"`; - -exports[`Stringify repeat 1`] = `"{\\"data\\":{\\"_value\\":\\"hi\\",\\"size\\":100},\\"__serializedType__\\":\\"ImmutableRepeat\\"}"`; - -exports[`Stringify seq 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8],\\"__serializedType__\\":\\"ImmutableSeq\\"}"`; - -exports[`Stringify set 1`] = `"{\\"data\\":[1,2,3,4,5,6,7,8,9,10],\\"__serializedType__\\":\\"ImmutableSet\\"}"`; - -exports[`Stringify stack 1`] = `"{\\"data\\":[\\"a\\",\\"b\\",\\"c\\"],\\"__serializedType__\\":\\"ImmutableStack\\"}"`; - exports[`Stringify uploadedAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2\\",\\"__serializedType__\\":\\"UploadedAvatarURL\\"}"`; exports[`Stringify url 1`] = `"{\\"data\\":\\"https://chat.zulip.org/\\",\\"__serializedType__\\":\\"URL\\"}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index d26f49a9a79..e040beb5910 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -11,14 +11,6 @@ import * as eg from '../../__tests__/lib/exampleData'; const data = { map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), - orderedMap: Immutable.OrderedMap({ b: 2, a: 1, c: 3, d: 4 }), - list: Immutable.List([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), - range: Immutable.Range(0, 7), - repeat: Immutable.Repeat('hi', 100), - set: Immutable.Set([10, 9, 8, 7, 6, 5, 4, 3, 2, 1]), - orderedSet: Immutable.OrderedSet([10, 9, 8, 7, 6, 5, 4, 3, 2, 1]), - seq: Immutable.Seq.Indexed.of(1, 2, 3, 4, 5, 6, 7, 8), - stack: Immutable.Stack.of('a', 'b', 'c'), zulipVersion: new ZulipVersion('3.0.0'), url: new URL('https://chat.zulip.org'), gravatarURL: GravatarURL.validateAndConstructInstance({ email: eg.selfUser.email }), diff --git a/src/boot/store.js b/src/boot/store.js index 6935603cf3c..307872735e3 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -15,7 +15,7 @@ import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; import jsan from 'jsan'; -import { mark, extract, refer } from 'remotedev-serialize/helpers'; +import { mark } from 'remotedev-serialize/helpers'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -409,36 +409,9 @@ const customReviver = (key, value, defaultReviver) => { // change over the next few commits. const serialize = function serialize(_Immutable, refs, _customReplacer, _customReviver) { function replacer(key, value) { - if (value instanceof _Immutable.Record) { - return refer(value, 'ImmutableRecord', 'toObject', refs); - } - if (value instanceof _Immutable.Range) { - return extract(value, 'ImmutableRange'); - } - if (value instanceof _Immutable.Repeat) { - return extract(value, 'ImmutableRepeat'); - } - if (_Immutable.OrderedMap.isOrderedMap(value)) { - return mark(value, 'ImmutableOrderedMap', 'toObject'); - } if (_Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); } - if (_Immutable.List.isList(value)) { - return mark(value, 'ImmutableList', 'toArray'); - } - if (_Immutable.OrderedSet.isOrderedSet(value)) { - return mark(value, 'ImmutableOrderedSet', 'toArray'); - } - if (_Immutable.Set.isSet(value)) { - return mark(value, 'ImmutableSet', 'toArray'); - } - if (_Immutable.Seq.isSeq(value)) { - return mark(value, 'ImmutableSeq', 'toArray'); - } - if (_Immutable.Stack.isStack(value)) { - return mark(value, 'ImmutableStack', 'toArray'); - } if (typeof value === 'object' && value !== null && '__serializedType__' in value) { const copy = { ...value }; delete copy.__serializedType__; @@ -457,26 +430,6 @@ const serialize = function serialize(_Immutable, refs, _customReplacer, _customR switch (value.__serializedType__) { case 'ImmutableMap': return _Immutable.Map(data); - case 'ImmutableOrderedMap': - return _Immutable.OrderedMap(data); - case 'ImmutableList': - return _Immutable.List(data); - case 'ImmutableRange': - return _Immutable.Range(data._start, data._end, data._step); - case 'ImmutableRepeat': - return _Immutable.Repeat(data._value, data.size); - case 'ImmutableSet': - return _Immutable.Set(data); - case 'ImmutableOrderedSet': - return _Immutable.OrderedSet(data); - case 'ImmutableSeq': - return _Immutable.Seq(data); - case 'ImmutableStack': - return _Immutable.Stack(data); - case 'ImmutableRecord': - return refs && refs[value.__serializedRef__] - ? new refs[value.__serializedRef__](data) - : _Immutable.Map(data); case 'Object': return { ...data, __serializedType__: value.__serializedType__value }; default: From 5c61196f939792a1d39f88c215d6b3ae3dfcdc4e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:37:36 -0500 Subject: [PATCH 23/46] store [nfc]: Inline remotedev-serialize (4/x). This param stopped being used in the previous commit, so, remove it. --- src/boot/store.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 307872735e3..73c0cac5951 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -407,7 +407,7 @@ const customReviver = (key, value, defaultReviver) => { // Recently inlined from // node_modules/remotedev-serialize/immutable/serialize.js; this will // change over the next few commits. -const serialize = function serialize(_Immutable, refs, _customReplacer, _customReviver) { +const serialize = function serialize(_Immutable, _customReplacer, _customReviver) { function replacer(key, value) { if (_Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); @@ -461,7 +461,7 @@ const serialize = function serialize(_Immutable, refs, _customReplacer, _customR export const stringify = function (data: mixed): string { return jsan.stringify( data, - serialize(Immutable, null, customReplacer, customReviver).replacer, + serialize(Immutable, customReplacer, customReviver).replacer, null, jsanOptions, ); @@ -472,7 +472,7 @@ export const stringify = function (data: mixed): string { // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. export const parse = function (data: string): mixed { - return jsan.parse(data, serialize(Immutable, null, customReplacer, customReviver).reviver); + return jsan.parse(data, serialize(Immutable, customReplacer, customReviver).reviver); }; /** From b0fee363d1305bec64878faaa3eb59ddcf4428c0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:39:19 -0500 Subject: [PATCH 24/46] store [nfc]: Inline remotedev-serialize (5/x). The "false" branch of these conditionals won't ever be exercised. --- src/boot/store.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 73c0cac5951..68a2856c413 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -440,16 +440,12 @@ const serialize = function serialize(_Immutable, _customReplacer, _customReviver } return { - replacer: _customReplacer - ? function (key, value) { - return _customReplacer(key, value, replacer); - } - : replacer, - reviver: _customReviver - ? function (key, value) { - return _customReviver(key, value, reviver); - } - : reviver, + replacer(key, value) { + return _customReplacer(key, value, replacer); + }, + reviver(key, value) { + return _customReviver(key, value, reviver); + }, options: jsanOptions, }; }; From b9b3633264449401840b48a4c5a84493da5b625b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:40:51 -0500 Subject: [PATCH 25/46] store [nfc]: Inline remotedev-serialize (6/x). Continue dissolving the `serialize` factory by removing some unnecessary customization from its interface. --- src/boot/store.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 68a2856c413..dec591763e6 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -407,9 +407,9 @@ const customReviver = (key, value, defaultReviver) => { // Recently inlined from // node_modules/remotedev-serialize/immutable/serialize.js; this will // change over the next few commits. -const serialize = function serialize(_Immutable, _customReplacer, _customReviver) { +const serialize = function serialize() { function replacer(key, value) { - if (_Immutable.Map.isMap(value)) { + if (Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); } if (typeof value === 'object' && value !== null && '__serializedType__' in value) { @@ -429,7 +429,7 @@ const serialize = function serialize(_Immutable, _customReplacer, _customReviver const data = value.data; switch (value.__serializedType__) { case 'ImmutableMap': - return _Immutable.Map(data); + return Immutable.Map(data); case 'Object': return { ...data, __serializedType__: value.__serializedType__value }; default: @@ -441,10 +441,10 @@ const serialize = function serialize(_Immutable, _customReplacer, _customReviver return { replacer(key, value) { - return _customReplacer(key, value, replacer); + return customReplacer(key, value, replacer); }, reviver(key, value) { - return _customReviver(key, value, reviver); + return customReviver(key, value, reviver); }, options: jsanOptions, }; @@ -455,12 +455,7 @@ const serialize = function serialize(_Immutable, _customReplacer, _customReviver // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. export const stringify = function (data: mixed): string { - return jsan.stringify( - data, - serialize(Immutable, customReplacer, customReviver).replacer, - null, - jsanOptions, - ); + return jsan.stringify(data, serialize().replacer, null, jsanOptions); }; /** PRIVATE: Exported only for tests. */ @@ -468,7 +463,7 @@ export const stringify = function (data: mixed): string { // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. export const parse = function (data: string): mixed { - return jsan.parse(data, serialize(Immutable, customReplacer, customReviver).reviver); + return jsan.parse(data, serialize().reviver); }; /** From ed7a2135a56fcca50e14a1570a887dd88ef16372 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:44:23 -0500 Subject: [PATCH 26/46] store [nfc]: Inline remotedev-serialize (7/x). We don't use this bit of output from the `serialize` factory. --- src/boot/store.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/boot/store.js b/src/boot/store.js index dec591763e6..c25b9aa99ad 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -446,7 +446,6 @@ const serialize = function serialize() { reviver(key, value) { return customReviver(key, value, reviver); }, - options: jsanOptions, }; }; From d91001b06fdc2d656775aeccf3ed61c87b82e660 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:45:38 -0500 Subject: [PATCH 27/46] store [nfc]: Inline remotedev-serialize (8/x). Give these functions better names; we'll bring them out of the `serialize` factory soon, to help us along in dissolving `serialize`. --- src/boot/store.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index c25b9aa99ad..c96ea767fc8 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -408,7 +408,7 @@ const customReviver = (key, value, defaultReviver) => { // node_modules/remotedev-serialize/immutable/serialize.js; this will // change over the next few commits. const serialize = function serialize() { - function replacer(key, value) { + function defaultReplacer(key, value) { if (Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); } @@ -424,7 +424,7 @@ const serialize = function serialize() { return value; } - function reviver(key, value) { + function defaultReviver(key, value) { if (typeof value === 'object' && value !== null && '__serializedType__' in value) { const data = value.data; switch (value.__serializedType__) { @@ -441,10 +441,10 @@ const serialize = function serialize() { return { replacer(key, value) { - return customReplacer(key, value, replacer); + return customReplacer(key, value, defaultReplacer); }, reviver(key, value) { - return customReviver(key, value, reviver); + return customReviver(key, value, defaultReviver); }, }; }; From a0dcf4fdffef73790c5447ce2373e8e32b10aa8f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:48:28 -0500 Subject: [PATCH 28/46] store [nfc]: Inline remotedev-serialize (9/x). Further dissolve `serialize` by bringing `default{Replacer,Reviver}` into `custom{Replacer,Reviver}`'s lexical environment instead of passing them as arguments. Maintain that they're non-arrow functions (this is important for letting these functions take a special value of `this`, which we'll use in an upcoming commit), but make them function expressions assigned to same-named variables, instead of plain function declarations, so we don't have to think about hoisting (e.g., because store.js has gotten quite long). --- src/boot/store.js | 70 +++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index c96ea767fc8..dcd9dff74f6 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -362,7 +362,23 @@ const jsanOptions = { infinity: true, }; -const customReplacer = (key, value, defaultReplacer) => { +const defaultReplacer = function defaultReplacer(key, value) { + if (Immutable.Map.isMap(value)) { + return mark(value, 'ImmutableMap', 'toObject'); + } + if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + const copy = { ...value }; + delete copy.__serializedType__; + return { + __serializedType__: 'Object', + data: copy, + __serializedType__value: value.__serializedType__, + }; + } + return value; +}; + +const customReplacer = (key, value) => { if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; } else if (value instanceof URL) { @@ -383,7 +399,22 @@ const customReplacer = (key, value, defaultReplacer) => { return defaultReplacer(key, value); }; -const customReviver = (key, value, defaultReviver) => { +const defaultReviver = function defaultReviver(key, value) { + if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + const data = value.data; + switch (value.__serializedType__) { + case 'ImmutableMap': + return Immutable.Map(data); + case 'Object': + return { ...data, __serializedType__: value.__serializedType__value }; + default: + return data; + } + } + return value; +}; + +const customReviver = (key, value) => { if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { const data = value.data; switch (value[SERIALIZED_TYPE_FIELD_NAME]) { @@ -408,43 +439,12 @@ const customReviver = (key, value, defaultReviver) => { // node_modules/remotedev-serialize/immutable/serialize.js; this will // change over the next few commits. const serialize = function serialize() { - function defaultReplacer(key, value) { - if (Immutable.Map.isMap(value)) { - return mark(value, 'ImmutableMap', 'toObject'); - } - if (typeof value === 'object' && value !== null && '__serializedType__' in value) { - const copy = { ...value }; - delete copy.__serializedType__; - return { - __serializedType__: 'Object', - data: copy, - __serializedType__value: value.__serializedType__, - }; - } - return value; - } - - function defaultReviver(key, value) { - if (typeof value === 'object' && value !== null && '__serializedType__' in value) { - const data = value.data; - switch (value.__serializedType__) { - case 'ImmutableMap': - return Immutable.Map(data); - case 'Object': - return { ...data, __serializedType__: value.__serializedType__value }; - default: - return data; - } - } - return value; - } - return { replacer(key, value) { - return customReplacer(key, value, defaultReplacer); + return customReplacer(key, value); }, reviver(key, value) { - return customReviver(key, value, defaultReviver); + return customReviver(key, value); }, }; }; From 27598f911ef4caeb5620cbcff617ef35def7fe20 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 12:58:13 -0500 Subject: [PATCH 29/46] store [nfc]: Inline remotedev-serialize (10/x). Finish dissolving the `serialize` factory. Maintain that `replacer` and `reviver` are non-arrow functions (this is important for letting these functions take a special value of `this`, which we'll use in an upcoming commit). --- src/boot/store.js | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index dcd9dff74f6..16b55020695 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -399,6 +399,10 @@ const customReplacer = (key, value) => { return defaultReplacer(key, value); }; +const replacer = function replacer(key, value) { + return customReplacer(key, value); +}; + const defaultReviver = function defaultReviver(key, value) { if (typeof value === 'object' && value !== null && '__serializedType__' in value) { const data = value.data; @@ -435,18 +439,8 @@ const customReviver = (key, value) => { return defaultReviver(key, value); }; -// Recently inlined from -// node_modules/remotedev-serialize/immutable/serialize.js; this will -// change over the next few commits. -const serialize = function serialize() { - return { - replacer(key, value) { - return customReplacer(key, value); - }, - reviver(key, value) { - return customReviver(key, value); - }, - }; +const reviver = function reviver(key, value) { + return customReviver(key, value); }; /** PRIVATE: Exported only for tests. */ @@ -454,7 +448,7 @@ const serialize = function serialize() { // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. export const stringify = function (data: mixed): string { - return jsan.stringify(data, serialize().replacer, null, jsanOptions); + return jsan.stringify(data, replacer, null, jsanOptions); }; /** PRIVATE: Exported only for tests. */ @@ -462,7 +456,7 @@ export const stringify = function (data: mixed): string { // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. export const parse = function (data: string): mixed { - return jsan.parse(data, serialize().reviver); + return jsan.parse(data, reviver); }; /** From 460f27b414610153a3d54dcd5ac854d32b310d1c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 13:01:31 -0500 Subject: [PATCH 30/46] store [nfc]: Make `customReplacer` and `customReviver` non-arrow functions. This is only NFC because we haven't yet been using `this` in `customReplacer` or `customReviver`. The `replacer` function that's passed to `JSON.stringify`, or `jsan.stringify`, is called with an interesting value as `this`: the "holder" object, as JSON.stringify walks over the entire object. Or, at least, it does so as long as the `replacer` function is a non-arrow function. For an arrow function, `this` is set based on the lexical scope in which it was defined. remotedev-serialize has always taken care to pass a non-arrow function in the one place it calls `jsan.stringify`. But then it calls our code, `customReplacer` -- and we get to decide whether we want `customReplacer`'s `this` to have that interesting value. So far, we've decided we don't, by making `customReplacer` an arrow function. If we make it a non-arrow function, its `this` will be the `this` of its caller. That decision doesn't seem well-considered; and, in particular, we'll start using that interesting value in an upcoming commit. The story is very similar for the `reviver` passed to `JSON.stringify` / `jsan.stringify`. --- src/boot/store.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 16b55020695..5a377a5ce54 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -378,7 +378,7 @@ const defaultReplacer = function defaultReplacer(key, value) { return value; }; -const customReplacer = (key, value) => { +const customReplacer = function customReplacer(key, value) { if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; } else if (value instanceof URL) { @@ -418,7 +418,7 @@ const defaultReviver = function defaultReviver(key, value) { return value; }; -const customReviver = (key, value) => { +const customReviver = function customReviver(key, value) { if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { const data = value.data; switch (value[SERIALIZED_TYPE_FIELD_NAME]) { From 1376e1e8c1d58df63d29d503d8af44cca1f29216 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 13:34:38 -0500 Subject: [PATCH 31/46] store [nfc]: Inline remotedev-serialize (11/x). Bring `{custom,default}{Replacer,Reviver}` into `replacer` and `reviver`. Soon, we'll further remove the distinction between "default" and "custom", which had some meaning when this code wasn't totally ours, but doesn't now. --- src/boot/store.js | 66 ++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 5a377a5ce54..a066f75f7b2 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -362,23 +362,8 @@ const jsanOptions = { infinity: true, }; -const defaultReplacer = function defaultReplacer(key, value) { - if (Immutable.Map.isMap(value)) { - return mark(value, 'ImmutableMap', 'toObject'); - } - if (typeof value === 'object' && value !== null && '__serializedType__' in value) { - const copy = { ...value }; - delete copy.__serializedType__; - return { - __serializedType__: 'Object', - data: copy, - __serializedType__value: value.__serializedType__, - }; - } - return value; -}; - -const customReplacer = function customReplacer(key, value) { +const replacer = function replacer(key, value) { + // customReplacer, previously if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; } else if (value instanceof URL) { @@ -396,29 +381,25 @@ const customReplacer = function customReplacer(key, value) { [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; } - return defaultReplacer(key, value); -}; -const replacer = function replacer(key, value) { - return customReplacer(key, value); -}; - -const defaultReviver = function defaultReviver(key, value) { + // defaultReplacer, previously + if (Immutable.Map.isMap(value)) { + return mark(value, 'ImmutableMap', 'toObject'); + } if (typeof value === 'object' && value !== null && '__serializedType__' in value) { - const data = value.data; - switch (value.__serializedType__) { - case 'ImmutableMap': - return Immutable.Map(data); - case 'Object': - return { ...data, __serializedType__: value.__serializedType__value }; - default: - return data; - } + const copy = { ...value }; + delete copy.__serializedType__; + return { + __serializedType__: 'Object', + data: copy, + __serializedType__value: value.__serializedType__, + }; } return value; }; -const customReviver = function customReviver(key, value) { +const reviver = function reviver(key, value) { + // customReviver, previously if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { const data = value.data; switch (value[SERIALIZED_TYPE_FIELD_NAME]) { @@ -436,11 +417,20 @@ const customReviver = function customReviver(key, value) { // Fall back to defaultReviver, below } } - return defaultReviver(key, value); -}; -const reviver = function reviver(key, value) { - return customReviver(key, value); + // defaultReviver, previously + if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + const data = value.data; + switch (value.__serializedType__) { + case 'ImmutableMap': + return Immutable.Map(data); + case 'Object': + return { ...data, __serializedType__: value.__serializedType__value }; + default: + return data; + } + } + return value; }; /** PRIVATE: Exported only for tests. */ From 713c95add07be9555b8394c9bef5e94e07940a5f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 14:33:37 -0500 Subject: [PATCH 32/46] store [nfc]: Inline remotedev-serialize (12/x). Use our constant for all these instances, so we don't have to retype the string literal each time (risking misspelling it). --- src/boot/__tests__/replaceRevive-test.js | 6 ++--- src/boot/store.js | 29 +++++++++++++----------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index e040beb5910..fc7a43d10d3 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -6,7 +6,7 @@ import Immutable from 'immutable'; import { FallbackAvatarURL, GravatarURL, UploadedAvatarURL } from '../../utils/avatar'; import { ZulipVersion } from '../../utils/zulipVersion'; -import { stringify, parse } from '../store'; +import { stringify, parse, SERIALIZED_TYPE_FIELD_NAME } from '../store'; import * as eg from '../../__tests__/lib/exampleData'; const data = { @@ -25,9 +25,9 @@ const data = { }), withTypeKey: { a: 1, - __serializedType__: { + [SERIALIZED_TYPE_FIELD_NAME]: { b: [2], - __serializedType__: { c: [3] }, + [SERIALIZED_TYPE_FIELD_NAME]: { c: [3] }, }, }, }; diff --git a/src/boot/store.js b/src/boot/store.js index a066f75f7b2..6aa608c51b1 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -3,8 +3,6 @@ // Switch off some rules for this file as we continue folding // remotedev-serialize into our code; we'll remove these soon. // -/* eslint-disable no-underscore-dangle */ -/* eslint-disable id-match */ /* eslint-disable func-names */ /* flowlint untyped-import:off */ @@ -338,12 +336,14 @@ provideLoggingContext(() => ({ })); /** - * A special identifier used by `remotedev-serialize`. + * PRIVATE: Exported only for tests. * - * Use this in the custom replacer and reviver, below, to make it - * easier to be consistent between them and avoid costly typos. + * A special identifier for the type of thing to be replaced/revived. + * + * Use this in the replacer and reviver, below, to make it easier to + * be consistent between them and avoid costly typos. */ -const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; +export const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; // Recently inlined from // node_modules/remotedev-serialize/constants/options.js; this will @@ -386,13 +386,13 @@ const replacer = function replacer(key, value) { if (Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); } - if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; - delete copy.__serializedType__; + delete copy[SERIALIZED_TYPE_FIELD_NAME]; return { - __serializedType__: 'Object', + [SERIALIZED_TYPE_FIELD_NAME]: 'Object', data: copy, - __serializedType__value: value.__serializedType__, + [`${SERIALIZED_TYPE_FIELD_NAME}value`]: value[SERIALIZED_TYPE_FIELD_NAME], }; } return value; @@ -419,13 +419,16 @@ const reviver = function reviver(key, value) { } // defaultReviver, previously - if (typeof value === 'object' && value !== null && '__serializedType__' in value) { + if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const data = value.data; - switch (value.__serializedType__) { + switch (value[SERIALIZED_TYPE_FIELD_NAME]) { case 'ImmutableMap': return Immutable.Map(data); case 'Object': - return { ...data, __serializedType__: value.__serializedType__value }; + return { + ...data, + [SERIALIZED_TYPE_FIELD_NAME]: value[`${SERIALIZED_TYPE_FIELD_NAME}value`], + }; default: return data; } From e2701d64f0e451e70d717b236cfb248c708416dc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 14:39:17 -0500 Subject: [PATCH 33/46] store [nfc]: Inline remotedev-serialize (13/x). Might as well have a constant for this one, too. --- src/boot/store.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 6aa608c51b1..115a9377ca0 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -345,6 +345,14 @@ provideLoggingContext(() => ({ */ export const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; +/** + * Like SERIALIZED_TYPE_FIELD_NAME, but with a distinguishing mark. + * + * Used in our strategy to ensure successful round-tripping when data + * has a key identical to SERIALIZED_TYPE_FIELD_NAME. + */ +const SERIALIZED_TYPE_FIELD_NAME_ESCAPED: '__serializedType__value' = '__serializedType__value'; + // Recently inlined from // node_modules/remotedev-serialize/constants/options.js; this will // change over the next few commits. @@ -392,7 +400,7 @@ const replacer = function replacer(key, value) { return { [SERIALIZED_TYPE_FIELD_NAME]: 'Object', data: copy, - [`${SERIALIZED_TYPE_FIELD_NAME}value`]: value[SERIALIZED_TYPE_FIELD_NAME], + [SERIALIZED_TYPE_FIELD_NAME_ESCAPED]: value[SERIALIZED_TYPE_FIELD_NAME], }; } return value; @@ -427,7 +435,7 @@ const reviver = function reviver(key, value) { case 'Object': return { ...data, - [SERIALIZED_TYPE_FIELD_NAME]: value[`${SERIALIZED_TYPE_FIELD_NAME}value`], + [SERIALIZED_TYPE_FIELD_NAME]: value[SERIALIZED_TYPE_FIELD_NAME_ESCAPED], }; default: return data; From 014487692dc5de31911d4e5ac4dee04e347fc6c3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 14:45:28 -0500 Subject: [PATCH 34/46] store [nfc]: Inline remotedev-serialize (14/x). The conditions are the same, and the switch statements use the same expression, so we can consolidate these. Since we're inlining remotedev-serialize, there's no meaningful distinction between `customReviver` and `defaultReviver` anymore. --- src/boot/store.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 115a9377ca0..b80fda94b19 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -407,7 +407,6 @@ const replacer = function replacer(key, value) { }; const reviver = function reviver(key, value) { - // customReviver, previously if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { const data = value.data; switch (value[SERIALIZED_TYPE_FIELD_NAME]) { @@ -421,15 +420,6 @@ const reviver = function reviver(key, value) { return UploadedAvatarURL.deserialize(data); case 'FallbackAvatarURL': return FallbackAvatarURL.deserialize(data); - default: - // Fall back to defaultReviver, below - } - } - - // defaultReviver, previously - if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { - const data = value.data; - switch (value[SERIALIZED_TYPE_FIELD_NAME]) { case 'ImmutableMap': return Immutable.Map(data); case 'Object': From 25f147fc16ff04080a0df61a845afb4046ab408d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 14:49:45 -0500 Subject: [PATCH 35/46] store [nfc]: Inline remotedev-serialize (15/x). Like we did in the previous commit for `reviver`, consolidate the conditions in `replacer` so they look consistent. Since we're inlining remotedev-serialize, there's no meaningful distinction between `customReviver` and `defaultReviver` anymore. --- src/boot/store.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index b80fda94b19..1dbb75e14cb 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -371,7 +371,6 @@ const jsanOptions = { }; const replacer = function replacer(key, value) { - // customReplacer, previously if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; } else if (value instanceof URL) { @@ -388,13 +387,9 @@ const replacer = function replacer(key, value) { data: FallbackAvatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; - } - - // defaultReplacer, previously - if (Immutable.Map.isMap(value)) { + } else if (Immutable.Map.isMap(value)) { return mark(value, 'ImmutableMap', 'toObject'); - } - if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { + } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; return { From 8d3dd1e5adb349ad9cf048866eda1cdf666dc893 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 15:11:28 -0500 Subject: [PATCH 36/46] store [nfc]: Inline remotedev-serialize (16/x). Inline the `mark` helper function. Now we have no more imports from `remotedev-serialize`! --- src/boot/store.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 1dbb75e14cb..4fa62b5ef58 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -13,7 +13,6 @@ import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; import jsan from 'jsan'; -import { mark } from 'remotedev-serialize/helpers'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -388,7 +387,7 @@ const replacer = function replacer(key, value) { [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; } else if (Immutable.Map.isMap(value)) { - return mark(value, 'ImmutableMap', 'toObject'); + return { data: value.toObject(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; From 5838c653603c6df6b6f553174c97e35089ed9770 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 15:35:39 -0500 Subject: [PATCH 37/46] store [nfc]: Inline remotedev-serialize (17/x). ESLint's `func-names` wants these functions to be given names. Might as well; this may even help us make more sense of profiling data in Chrome's DevTools. --- src/boot/store.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 4fa62b5ef58..16c5912710e 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -3,7 +3,6 @@ // Switch off some rules for this file as we continue folding // remotedev-serialize into our code; we'll remove these soon. // -/* eslint-disable func-names */ /* flowlint untyped-import:off */ import { applyMiddleware, compose, createStore } from 'redux'; @@ -432,7 +431,7 @@ const reviver = function reviver(key, value) { // Recently inlined from // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. -export const stringify = function (data: mixed): string { +export const stringify = function stringify(data: mixed): string { return jsan.stringify(data, replacer, null, jsanOptions); }; @@ -440,7 +439,7 @@ export const stringify = function (data: mixed): string { // Recently inlined from // node_modules/remotedev-serialize/immutable/index.js; this will // change over the next few commits. -export const parse = function (data: string): mixed { +export const parse = function parse(data: string): mixed { return jsan.parse(data, reviver); }; From 757bd4024c4c101293d0e7321bd9bc8bec907756 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 14:52:06 -0500 Subject: [PATCH 38/46] deps: Remove remotedev-serialize. We've just inlined the code we were using from remotedev-serialize. Next, as mentioned in the first commit in the series that did that, we'd like to stop depending on `jsan`; we'll do that in the next few commits. --- flow-typed/remotedev-serialize_vx.x.x.js | 40 ------------------------ package.json | 1 - src/boot/store.js | 10 +----- yarn.lock | 8 +---- 4 files changed, 2 insertions(+), 57 deletions(-) delete mode 100644 flow-typed/remotedev-serialize_vx.x.x.js diff --git a/flow-typed/remotedev-serialize_vx.x.x.js b/flow-typed/remotedev-serialize_vx.x.x.js deleted file mode 100644 index 75193614549..00000000000 --- a/flow-typed/remotedev-serialize_vx.x.x.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * From DefinitelyTyped - * (https://github.com/DefinitelyTyped/DefinitelyTyped/blob/55ebcedca/types/remotedev-serialize/index.d.ts) - * via FlowGen v1.10.0, with minimal manual tweaks (run - * - * `git log --stat -p --full-diff -- flow-typed/remotedev-serialize_vx.x.x.js` - * - * for info). - */ -declare module 'remotedev-serialize' { - declare export type Options = { [key: string]: boolean, ... }; - declare export type Refs = { [key: string]: any, ... }; - declare export type DefaultReplacer = (key: string, value: any) => any; - declare export type Replacer = (key: string, value: any, replacer: DefaultReplacer) => any; - declare export type DefaultReviver = (key: string, value: any) => any; - declare export type Reviver = (key: string, value: any, reviver: DefaultReviver) => any; - declare export function immutable( - // This `any` is unavoidable; see - // https://github.com/flow-typed/flow-typed/blob/master/CONTRIBUTING.md#dont-import-types-from-other-libdefs. - immutable: any, - refs?: Refs | null, - customReplacer?: Replacer, - customReviver?: Reviver, - ): { - stringify: (input: any) => string, - parse: (input: string) => any, - serialize: ( - immutable: any, - refs?: Refs, - customReplacer?: Replacer, - customReviver?: Reviver, - ) => { - replacer: Replacer, - reviver: Reviver, - options: Options, - ... - }, - ... - }; -} diff --git a/package.json b/package.json index f578997ae78..3c2e932a58a 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,6 @@ "redux-batched-actions": "^0.3.0", "redux-logger": "^3.0.1", "redux-thunk": "^2.1.0", - "remotedev-serialize": "zulip/remotedev-serialize#5f9f759a4", "reselect": "^3.0.1", "rn-fetch-blob": "^0.11.0", "string.fromcodepoint": "^0.2.1", diff --git a/src/boot/store.js b/src/boot/store.js index 16c5912710e..9031debe89b 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -1,8 +1,6 @@ /* @flow strict-local */ -// Switch off some rules for this file as we continue folding -// remotedev-serialize into our code; we'll remove these soon. -// +// We'll remove `jsan` very soon. /* flowlint untyped-import:off */ import { applyMiddleware, compose, createStore } from 'redux'; @@ -428,17 +426,11 @@ const reviver = function reviver(key, value) { }; /** PRIVATE: Exported only for tests. */ -// Recently inlined from -// node_modules/remotedev-serialize/immutable/index.js; this will -// change over the next few commits. export const stringify = function stringify(data: mixed): string { return jsan.stringify(data, replacer, null, jsanOptions); }; /** PRIVATE: Exported only for tests. */ -// Recently inlined from -// node_modules/remotedev-serialize/immutable/index.js; this will -// change over the next few commits. export const parse = function parse(data: string): mixed { return jsan.parse(data, reviver); }; diff --git a/yarn.lock b/yarn.lock index dfd16cf018b..d7a0d05ee36 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7669,7 +7669,7 @@ js-yaml@^3.13.0, js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -jsan@3.1.13, jsan@^3.1.13: +jsan@3.1.13: version "3.1.13" resolved "https://registry.yarnpkg.com/jsan/-/jsan-3.1.13.tgz#4de8c7bf8d1cfcd020c313d438f930cec4b91d86" integrity sha512-9kGpCsGHifmw6oJet+y8HaCl14y7qgAsxVdV3pCHDySNR3BfDC30zgkssd7x5LRVAT22dnpbe9JdzzmXZnq9/g== @@ -10524,12 +10524,6 @@ regjsparser@^0.6.4: dependencies: jsesc "~0.5.0" -remotedev-serialize@zulip/remotedev-serialize#5f9f759a4: - version "0.1.8" - resolved "https://codeload.github.com/zulip/remotedev-serialize/tar.gz/5f9f759a4f82821aedfd23aea686e1f784750189" - dependencies: - jsan "^3.1.13" - remove-trailing-separator@^1.0.1: version "1.1.0" resolved "https://registry.yarnpkg.com/remove-trailing-separator/-/remove-trailing-separator-1.1.0.tgz#c24bce2a283adad5bc3f58e0d48249b92379d8ef" From 8f6a63a742274892e718ed2b7c534fa0e6612bfe Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 15:50:40 -0500 Subject: [PATCH 39/46] store: Opt out of several `jsan` features we don't need. This will probably give us some performance boost in our replace/revive logic. A tempting way to make a change like this would be to pass `false` instead of this whole object -- but, examining the implementation, that would effectively opt us back into the `refs` option, which remotedev-serialize specifically wanted to set to false. --- src/boot/store.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 9031debe89b..be058ec49f5 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -354,16 +354,16 @@ const SERIALIZED_TYPE_FIELD_NAME_ESCAPED: '__serializedType__value' = '__seriali // change over the next few commits. const jsanOptions = { refs: false, // references can't be resolved on the original Immutable structure - date: true, - function: true, - regex: true, - undefined: true, - error: true, - symbol: true, - map: true, - set: true, - nan: true, - infinity: true, + date: false, + function: false, + regex: false, + undefined: false, + error: false, + symbol: false, + map: false, + set: false, + nan: false, + infinity: false, }; const replacer = function replacer(key, value) { From c3f18208451004cdae8df2e9c3265ba7dc036dc4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 16:16:53 -0500 Subject: [PATCH 40/46] store: Use JSON instead of `jsan` for replace/revive logic. The tricky part here is that, while we weren't relying on any of the advertised extra features of `jsan`, we were relying on it to recurse through our data and apply our `replacer` function [1]. JSON.stringify invariably calls the value's `.toJSON` method before the supplied `replacer` function gets to have any say about how the value should be serialized [2]. But Greg found a way to access the value before the `.toJSON` transformation, and it's quite simple to do with `JSON.stringify`'s API: we use `this`, which is the "holder" object, and access the `key` on it. The two things we use that have `toJSON` methods are `Immutable.Map` (which makes an object-as-map of the data) and `URL` (which gives the string representation). So, use `this[key]` to grab the pre-`toJSON` values for those when we do our `instanceof` checks for those. All the other classes we check with `instanceof` clearly don't have a `toJSON` method; we know this because we wrote them. Also, add a test where `__serializedType__` is a key in an Immutable.Map. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.60remotedev-serialize.60.20and.20.60jsan.60/near/1082842 [2] https://tc39.es/ecma262/#sec-serializejsonproperty (See steps 2 and 3.) --- .../__snapshots__/replaceRevive-test.js.snap | 6 +- src/boot/__tests__/replaceRevive-test.js | 9 ++- src/boot/store.js | 64 ++++++++++--------- 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap index 0ddb600885d..1da54cef8db 100644 --- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap +++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap @@ -6,10 +6,12 @@ exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.co exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; +exports[`Stringify mapWithTypeKey 1`] = `"{\\"data\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}},\\"__serializedType__\\":\\"ImmutableMap\\"}"`; + +exports[`Stringify plainObjectWithTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; + exports[`Stringify uploadedAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/user_avatars/2/e35cdbc4771c5e4b94e705bf6ff7cca7fa1efcae.png?x=x&version=2\\",\\"__serializedType__\\":\\"UploadedAvatarURL\\"}"`; exports[`Stringify url 1`] = `"{\\"data\\":\\"https://chat.zulip.org/\\",\\"__serializedType__\\":\\"URL\\"}"`; -exports[`Stringify withTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`; - exports[`Stringify zulipVersion 1`] = `"{\\"data\\":\\"3.0.0\\",\\"__serializedType__\\":\\"ZulipVersion\\"}"`; diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index fc7a43d10d3..647ca61a288 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -11,6 +11,13 @@ import * as eg from '../../__tests__/lib/exampleData'; const data = { map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }), + mapWithTypeKey: Immutable.Map({ + a: 1, + [SERIALIZED_TYPE_FIELD_NAME]: { + b: [2], + [SERIALIZED_TYPE_FIELD_NAME]: { c: [3] }, + }, + }), zulipVersion: new ZulipVersion('3.0.0'), url: new URL('https://chat.zulip.org'), gravatarURL: GravatarURL.validateAndConstructInstance({ email: eg.selfUser.email }), @@ -23,7 +30,7 @@ const data = { realm: eg.realm, userId: 1, }), - withTypeKey: { + plainObjectWithTypeKey: { a: 1, [SERIALIZED_TYPE_FIELD_NAME]: { b: [2], diff --git a/src/boot/store.js b/src/boot/store.js index be058ec49f5..987e4d3688b 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -1,15 +1,10 @@ /* @flow strict-local */ - -// We'll remove `jsan` very soon. -/* flowlint untyped-import:off */ - import { applyMiddleware, compose, createStore } from 'redux'; import type { Store } from 'redux'; import thunkMiddleware from 'redux-thunk'; import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; import Immutable from 'immutable'; -import jsan from 'jsan'; import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; @@ -349,28 +344,27 @@ export const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedTyp */ const SERIALIZED_TYPE_FIELD_NAME_ESCAPED: '__serializedType__value' = '__serializedType__value'; -// Recently inlined from -// node_modules/remotedev-serialize/constants/options.js; this will -// change over the next few commits. -const jsanOptions = { - refs: false, // references can't be resolved on the original Immutable structure - date: false, - function: false, - regex: false, - undefined: false, - error: false, - symbol: false, - map: false, - set: false, - nan: false, - infinity: false, -}; - +// Don't make this an arrow function -- we need `this` to be a special +// value; see +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter. const replacer = function replacer(key, value) { + // The value at the current path before JSON.stringify called its + // `toJSON` method, if present. + // + // When identifying what kind of thing we're working with, be sure + // to examine `origValue` instead of `value`, if calling `toJSON` on + // that kind of thing would remove its identifying features -- which + // is to say, if that kind of thing has a `toJSON` method. + // + // For things that have a `toJSON` method, it may be convenient to + // set `data` to `value`, if we trust that `toJSON` gives the output + // we want to store there. And it would mean we don't discard the + // work `JSON.stringify` did by calling `toJSON`. + const origValue = this[key]; if (value instanceof ZulipVersion) { return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; - } else if (value instanceof URL) { - return { data: value.toString(), [SERIALIZED_TYPE_FIELD_NAME]: 'URL' }; + } else if (origValue instanceof URL) { + return { data: origValue.toString(), [SERIALIZED_TYPE_FIELD_NAME]: 'URL' }; } else if (value instanceof GravatarURL) { return { data: GravatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'GravatarURL' }; } else if (value instanceof UploadedAvatarURL) { @@ -383,8 +377,8 @@ const replacer = function replacer(key, value) { data: FallbackAvatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; - } else if (Immutable.Map.isMap(value)) { - return { data: value.toObject(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; + } else if (Immutable.Map.isMap(origValue)) { + return { data: origValue.toObject(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; @@ -424,15 +418,27 @@ const reviver = function reviver(key, value) { } return value; }; - /** PRIVATE: Exported only for tests. */ export const stringify = function stringify(data: mixed): string { - return jsan.stringify(data, replacer, null, jsanOptions); + const result = JSON.stringify(data, replacer); + if (result === undefined) { + // Flow says that the output for JSON.stringify could be + // undefined. From MDN: + // + // `JSON.stringify()` can return `undefined` when passing in + // "pure" values like `JSON.stringify(function(){})` or + // `JSON.stringify(undefined)`. + // + // We don't expect any of those inputs, but we'd want to know if + // we get one, since it means something has gone quite wrong. + throw new Error('undefined result for stringify'); + } + return result; }; /** PRIVATE: Exported only for tests. */ export const parse = function parse(data: string): mixed { - return jsan.parse(data, reviver); + return JSON.parse(data, reviver); }; /** From 6a525728b2395d17929d023716081725c1575c80 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 16:26:12 -0500 Subject: [PATCH 41/46] deps: Remove `jsan`. We just removed our use of this in the previous commit. --- package.json | 1 - yarn.lock | 5 ----- 2 files changed, 6 deletions(-) diff --git a/package.json b/package.json index 3c2e932a58a..e091ac682c8 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "expo-splash-screen": "^0.5.0", "immutable": "^4.0.0-rc.12", "invariant": "^2.2.4", - "jsan": "3.1.13", "json-stringify-safe": "^5.0.1", "katex": "^0.11.1", "lodash.escape": "^4.0.1", diff --git a/yarn.lock b/yarn.lock index d7a0d05ee36..dc7945f69e3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7669,11 +7669,6 @@ js-yaml@^3.13.0, js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -jsan@3.1.13: - version "3.1.13" - resolved "https://registry.yarnpkg.com/jsan/-/jsan-3.1.13.tgz#4de8c7bf8d1cfcd020c313d438f930cec4b91d86" - integrity sha512-9kGpCsGHifmw6oJet+y8HaCl14y7qgAsxVdV3pCHDySNR3BfDC30zgkssd7x5LRVAT22dnpbe9JdzzmXZnq9/g== - jsbn@~0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/jsbn/-/jsbn-0.1.1.tgz#a5e654c2e5a2deb5f201d96cefbca80c0ef2f513" From 231d24b0276b18133c753757fcdb53683146aaa4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 16:37:59 -0500 Subject: [PATCH 42/46] store [nfc]: Use `origValue.toJSON()` instead of `.toObject()`. `toJSON` is in fact an alias for `toObject`, as Greg pointed out in chat [1]; see also node_modules/immutable/dist/immutable.js: ``` KeyedCollectionPrototype.toJSON = toObject; ``` In the next commit, we'll use `value` instead of `origValue.toJSON()`; JSON stringify will have already gone and called `toJSON` and given it to us as `value`, so we'll save a bit of work there. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.60remotedev-serialize.60.20and.20.60jsan.60/near/1082848 --- src/boot/store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/store.js b/src/boot/store.js index 987e4d3688b..64ebebbf0e7 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -378,7 +378,7 @@ const replacer = function replacer(key, value) { [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; } else if (Immutable.Map.isMap(origValue)) { - return { data: origValue.toObject(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; + return { data: origValue.toJSON(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; From bf8837c4ab03a70df38b6dcdc9a01177fffccd2b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 18 Dec 2020 16:41:37 -0500 Subject: [PATCH 43/46] store: Save a bit of work when serializing `Immutable.Map`s. As mentioned in the previous commit, JSON.stringify has already handed us the result of calling `.toJSON`, at `value`. So, save some work and stop calling it ourselves. --- src/boot/store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/store.js b/src/boot/store.js index 64ebebbf0e7..9aed5ce6fc6 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -378,7 +378,7 @@ const replacer = function replacer(key, value) { [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', }; } else if (Immutable.Map.isMap(origValue)) { - return { data: origValue.toJSON(), [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; + return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; From f985ef00a4eeeaf4ce01569bfaa166ca9cd29cc7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 15:14:34 -0500 Subject: [PATCH 44/46] store: Add some invariants to `stringify`. As Greg suggests at https://github.com/zulip/zulip-mobile/pull/4348#pullrequestreview-555904548. Put the invariants before the SERIALIZED_TYPE_FIELD_NAME checks; as Greg points out [1], they're logically no less applicable if the object happens to have a property with name SERIALIZED_TYPE_FIELD_NAME. [1] https://github.com/zulip/zulip-mobile/pull/4348#discussion_r548765084 --- src/boot/__tests__/replaceRevive-test.js | 18 ++++++++++++++++ src/boot/store.js | 27 +++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 647ca61a288..5a7e94d86f6 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -47,6 +47,24 @@ describe('Stringify', () => { expect(stringified[key]).toMatchSnapshot(); }); }); + + test('catches an unexpectedly unhandled value with a `toJSON` method', () => { + expect(() => stringify(new Date())).toThrow(); + }); + + test("catches a value that's definitely not serializable as-is", () => { + expect(() => stringify(() => 'foo')).toThrow(); + }); + + test('catches an unexpectedly unhandled value of an interesting type', () => { + class Dog { + noise = 'woof'; + makeNoise() { + console.log(this.noise); // eslint-disable-line no-console + } + } + expect(() => stringify(new Dog())).toThrow(); + }); }); describe('Parse', () => { diff --git a/src/boot/store.js b/src/boot/store.js index 9aed5ce6fc6..85cf6367e41 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -1,4 +1,5 @@ /* @flow strict-local */ +import invariant from 'invariant'; import { applyMiddleware, compose, createStore } from 'redux'; import type { Store } from 'redux'; import thunkMiddleware from 'redux-thunk'; @@ -379,7 +380,30 @@ const replacer = function replacer(key, value) { }; } else if (Immutable.Map.isMap(origValue)) { return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; - } else if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { + } + + if (typeof origValue === 'object' && origValue !== null) { + // Don't forget to handle a value's `toJSON` method, if present, as + // described above. + invariant(typeof origValue.toJSON !== 'function', 'unexpected toJSON'); + + // If storing an interesting data type, don't forget to handle it + // here, and in `reviver`. + const origValuePrototype = Object.getPrototypeOf(origValue); + invariant( + origValuePrototype + // "property `prototype` is missing in statics of `Object`" + // $FlowFixMe + === Object.prototype + || origValuePrototype + // "property `prototype` is missing in statics of `Array`" + // $FlowFixMe + === Array.prototype, + 'unexpected class', + ); + } + + if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { const copy = { ...value }; delete copy[SERIALIZED_TYPE_FIELD_NAME]; return { @@ -388,6 +412,7 @@ const replacer = function replacer(key, value) { [SERIALIZED_TYPE_FIELD_NAME_ESCAPED]: value[SERIALIZED_TYPE_FIELD_NAME], }; } + return value; }; From 44aa9e120c1164205c3822230747dad802cb2fa9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Dec 2020 15:18:59 -0500 Subject: [PATCH 45/46] replaceRevive [nfc]: Move replace-revive logic into its own file. It would have been better to do this at the start of this series, for a few reasons [1], but it would be annoying to redo the branch that way by brute force. [1] https://github.com/zulip/zulip-mobile/pull/4348#pullrequestreview-555904548 --- src/boot/__tests__/replaceRevive-test.js | 2 +- src/boot/replaceRevive.js | 144 +++++++++++++++++++++++ src/boot/store.js | 142 +--------------------- 3 files changed, 146 insertions(+), 142 deletions(-) create mode 100644 src/boot/replaceRevive.js diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js index 5a7e94d86f6..ad3d95630a8 100644 --- a/src/boot/__tests__/replaceRevive-test.js +++ b/src/boot/__tests__/replaceRevive-test.js @@ -6,7 +6,7 @@ import Immutable from 'immutable'; import { FallbackAvatarURL, GravatarURL, UploadedAvatarURL } from '../../utils/avatar'; import { ZulipVersion } from '../../utils/zulipVersion'; -import { stringify, parse, SERIALIZED_TYPE_FIELD_NAME } from '../store'; +import { stringify, parse, SERIALIZED_TYPE_FIELD_NAME } from '../replaceRevive'; import * as eg from '../../__tests__/lib/exampleData'; const data = { diff --git a/src/boot/replaceRevive.js b/src/boot/replaceRevive.js new file mode 100644 index 00000000000..ae552e83022 --- /dev/null +++ b/src/boot/replaceRevive.js @@ -0,0 +1,144 @@ +/* @flow strict-local */ +import invariant from 'invariant'; +import Immutable from 'immutable'; + +import { ZulipVersion } from '../utils/zulipVersion'; +import { GravatarURL, UploadedAvatarURL, FallbackAvatarURL } from '../utils/avatar'; + +/** + * PRIVATE: Exported only for tests. + * + * A special identifier for the type of thing to be replaced/revived. + * + * Use this in the replacer and reviver, below, to make it easier to + * be consistent between them and avoid costly typos. + */ +export const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; + +/** + * Like SERIALIZED_TYPE_FIELD_NAME, but with a distinguishing mark. + * + * Used in our strategy to ensure successful round-tripping when data + * has a key identical to SERIALIZED_TYPE_FIELD_NAME. + */ +const SERIALIZED_TYPE_FIELD_NAME_ESCAPED: '__serializedType__value' = '__serializedType__value'; + +// Don't make this an arrow function -- we need `this` to be a special +// value; see +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter. +const replacer = function replacer(key, value) { + // The value at the current path before JSON.stringify called its + // `toJSON` method, if present. + // + // When identifying what kind of thing we're working with, be sure + // to examine `origValue` instead of `value`, if calling `toJSON` on + // that kind of thing would remove its identifying features -- which + // is to say, if that kind of thing has a `toJSON` method. + // + // For things that have a `toJSON` method, it may be convenient to + // set `data` to `value`, if we trust that `toJSON` gives the output + // we want to store there. And it would mean we don't discard the + // work `JSON.stringify` did by calling `toJSON`. + const origValue = this[key]; + if (value instanceof ZulipVersion) { + return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; + } else if (origValue instanceof URL) { + return { data: origValue.toString(), [SERIALIZED_TYPE_FIELD_NAME]: 'URL' }; + } else if (value instanceof GravatarURL) { + return { data: GravatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'GravatarURL' }; + } else if (value instanceof UploadedAvatarURL) { + return { + data: UploadedAvatarURL.serialize(value), + [SERIALIZED_TYPE_FIELD_NAME]: 'UploadedAvatarURL', + }; + } else if (value instanceof FallbackAvatarURL) { + return { + data: FallbackAvatarURL.serialize(value), + [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', + }; + } else if (Immutable.Map.isMap(origValue)) { + return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; + } + + if (typeof origValue === 'object' && origValue !== null) { + // Don't forget to handle a value's `toJSON` method, if present, as + // described above. + invariant(typeof origValue.toJSON !== 'function', 'unexpected toJSON'); + + // If storing an interesting data type, don't forget to handle it + // here, and in `reviver`. + const origValuePrototype = Object.getPrototypeOf(origValue); + invariant( + origValuePrototype + // "property `prototype` is missing in statics of `Object`" + // $FlowFixMe + === Object.prototype + || origValuePrototype + // "property `prototype` is missing in statics of `Array`" + // $FlowFixMe + === Array.prototype, + 'unexpected class', + ); + } + + if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { + const copy = { ...value }; + delete copy[SERIALIZED_TYPE_FIELD_NAME]; + return { + [SERIALIZED_TYPE_FIELD_NAME]: 'Object', + data: copy, + [SERIALIZED_TYPE_FIELD_NAME_ESCAPED]: value[SERIALIZED_TYPE_FIELD_NAME], + }; + } + + return value; +}; + +const reviver = function reviver(key, value) { + if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { + const data = value.data; + switch (value[SERIALIZED_TYPE_FIELD_NAME]) { + case 'ZulipVersion': + return new ZulipVersion(data); + case 'URL': + return new URL(data); + case 'GravatarURL': + return GravatarURL.deserialize(data); + case 'UploadedAvatarURL': + return UploadedAvatarURL.deserialize(data); + case 'FallbackAvatarURL': + return FallbackAvatarURL.deserialize(data); + case 'ImmutableMap': + return Immutable.Map(data); + case 'Object': + return { + ...data, + [SERIALIZED_TYPE_FIELD_NAME]: value[SERIALIZED_TYPE_FIELD_NAME_ESCAPED], + }; + default: + return data; + } + } + return value; +}; + +export const stringify = function stringify(data: mixed): string { + const result = JSON.stringify(data, replacer); + if (result === undefined) { + // Flow says that the output for JSON.stringify could be + // undefined. From MDN: + // + // `JSON.stringify()` can return `undefined` when passing in + // "pure" values like `JSON.stringify(function(){})` or + // `JSON.stringify(undefined)`. + // + // We don't expect any of those inputs, but we'd want to know if + // we get one, since it means something has gone quite wrong. + throw new Error('undefined result for stringify'); + } + return result; +}; + +export const parse = function parse(data: string): mixed { + return JSON.parse(data, reviver); +}; diff --git a/src/boot/store.js b/src/boot/store.js index 85cf6367e41..09873c239db 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -1,5 +1,4 @@ /* @flow strict-local */ -import invariant from 'invariant'; import { applyMiddleware, compose, createStore } from 'redux'; import type { Store } from 'redux'; import thunkMiddleware from 'redux-thunk'; @@ -10,7 +9,7 @@ import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; import { ZulipVersion } from '../utils/zulipVersion'; -import { GravatarURL, UploadedAvatarURL, FallbackAvatarURL } from '../utils/avatar'; +import { stringify, parse } from './replaceRevive'; import type { Action, GlobalState } from '../types'; import config from '../config'; import { REHYDRATE } from '../actionConstants'; @@ -327,145 +326,6 @@ provideLoggingContext(() => ({ serverVersion: tryGetActiveAccount(store.getState())?.zulipVersion ?? null, })); -/** - * PRIVATE: Exported only for tests. - * - * A special identifier for the type of thing to be replaced/revived. - * - * Use this in the replacer and reviver, below, to make it easier to - * be consistent between them and avoid costly typos. - */ -export const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__'; - -/** - * Like SERIALIZED_TYPE_FIELD_NAME, but with a distinguishing mark. - * - * Used in our strategy to ensure successful round-tripping when data - * has a key identical to SERIALIZED_TYPE_FIELD_NAME. - */ -const SERIALIZED_TYPE_FIELD_NAME_ESCAPED: '__serializedType__value' = '__serializedType__value'; - -// Don't make this an arrow function -- we need `this` to be a special -// value; see -// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter. -const replacer = function replacer(key, value) { - // The value at the current path before JSON.stringify called its - // `toJSON` method, if present. - // - // When identifying what kind of thing we're working with, be sure - // to examine `origValue` instead of `value`, if calling `toJSON` on - // that kind of thing would remove its identifying features -- which - // is to say, if that kind of thing has a `toJSON` method. - // - // For things that have a `toJSON` method, it may be convenient to - // set `data` to `value`, if we trust that `toJSON` gives the output - // we want to store there. And it would mean we don't discard the - // work `JSON.stringify` did by calling `toJSON`. - const origValue = this[key]; - if (value instanceof ZulipVersion) { - return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' }; - } else if (origValue instanceof URL) { - return { data: origValue.toString(), [SERIALIZED_TYPE_FIELD_NAME]: 'URL' }; - } else if (value instanceof GravatarURL) { - return { data: GravatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'GravatarURL' }; - } else if (value instanceof UploadedAvatarURL) { - return { - data: UploadedAvatarURL.serialize(value), - [SERIALIZED_TYPE_FIELD_NAME]: 'UploadedAvatarURL', - }; - } else if (value instanceof FallbackAvatarURL) { - return { - data: FallbackAvatarURL.serialize(value), - [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', - }; - } else if (Immutable.Map.isMap(origValue)) { - return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' }; - } - - if (typeof origValue === 'object' && origValue !== null) { - // Don't forget to handle a value's `toJSON` method, if present, as - // described above. - invariant(typeof origValue.toJSON !== 'function', 'unexpected toJSON'); - - // If storing an interesting data type, don't forget to handle it - // here, and in `reviver`. - const origValuePrototype = Object.getPrototypeOf(origValue); - invariant( - origValuePrototype - // "property `prototype` is missing in statics of `Object`" - // $FlowFixMe - === Object.prototype - || origValuePrototype - // "property `prototype` is missing in statics of `Array`" - // $FlowFixMe - === Array.prototype, - 'unexpected class', - ); - } - - if (typeof value === 'object' && value !== null && SERIALIZED_TYPE_FIELD_NAME in value) { - const copy = { ...value }; - delete copy[SERIALIZED_TYPE_FIELD_NAME]; - return { - [SERIALIZED_TYPE_FIELD_NAME]: 'Object', - data: copy, - [SERIALIZED_TYPE_FIELD_NAME_ESCAPED]: value[SERIALIZED_TYPE_FIELD_NAME], - }; - } - - return value; -}; - -const reviver = function reviver(key, value) { - if (value !== null && typeof value === 'object' && SERIALIZED_TYPE_FIELD_NAME in value) { - const data = value.data; - switch (value[SERIALIZED_TYPE_FIELD_NAME]) { - case 'ZulipVersion': - return new ZulipVersion(data); - case 'URL': - return new URL(data); - case 'GravatarURL': - return GravatarURL.deserialize(data); - case 'UploadedAvatarURL': - return UploadedAvatarURL.deserialize(data); - case 'FallbackAvatarURL': - return FallbackAvatarURL.deserialize(data); - case 'ImmutableMap': - return Immutable.Map(data); - case 'Object': - return { - ...data, - [SERIALIZED_TYPE_FIELD_NAME]: value[SERIALIZED_TYPE_FIELD_NAME_ESCAPED], - }; - default: - return data; - } - } - return value; -}; -/** PRIVATE: Exported only for tests. */ -export const stringify = function stringify(data: mixed): string { - const result = JSON.stringify(data, replacer); - if (result === undefined) { - // Flow says that the output for JSON.stringify could be - // undefined. From MDN: - // - // `JSON.stringify()` can return `undefined` when passing in - // "pure" values like `JSON.stringify(function(){})` or - // `JSON.stringify(undefined)`. - // - // We don't expect any of those inputs, but we'd want to know if - // we get one, since it means something has gone quite wrong. - throw new Error('undefined result for stringify'); - } - return result; -}; - -/** PRIVATE: Exported only for tests. */ -export const parse = function parse(data: string): mixed { - return JSON.parse(data, reviver); -}; - /** * The config options to pass to redux-persist. * From 410f219a20d4f10a6a02ad08df9201828f423e8c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 28 Dec 2020 16:40:05 -0800 Subject: [PATCH 46/46] replaceRevive [nfc]: Identify the Flow bug causing the fixmes. --- src/boot/replaceRevive.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/boot/replaceRevive.js b/src/boot/replaceRevive.js index ae552e83022..c9f43746500 100644 --- a/src/boot/replaceRevive.js +++ b/src/boot/replaceRevive.js @@ -69,14 +69,9 @@ const replacer = function replacer(key, value) { // here, and in `reviver`. const origValuePrototype = Object.getPrototypeOf(origValue); invariant( - origValuePrototype - // "property `prototype` is missing in statics of `Object`" - // $FlowFixMe - === Object.prototype - || origValuePrototype - // "property `prototype` is missing in statics of `Array`" - // $FlowFixMe - === Array.prototype, + // Flow bug: https://github.com/facebook/flow/issues/6110 + origValuePrototype === (Object.prototype: $FlowFixMe) + || origValuePrototype === (Array.prototype: $FlowFixMe), 'unexpected class', ); }