Now deserializes object graphs with many references to future objects.#194
Now deserializes object graphs with many references to future objects.#194mressler wants to merge 1 commit intomobxjs:masterfrom
Conversation
| } else if (!this.hasError) { | ||
| fn(value!); | ||
| if (--this.pendingCallbacks === this.pendingRefsCount) { | ||
| if (--this.pendingCallbacks === 0) { |
There was a problem hiding this comment.
This is the heart of the change. I don't know why we were testing if these two seemingly dissimilar counts were equivalent. This happened to work in the nominal case when both are == 0, but did not work when pendingRefsCount hit a magic value during deserialization.
| it("it should not find supertypes", () => { | ||
| expect(() => { | ||
| deserialize(Store, { | ||
| foo = deserialize(Store, { |
There was a problem hiding this comment.
This was not throwing without assigning it to a variable. I don't know what JavaScript magic I invoked by assigning it to a variable, but here we go.
| "invalid", | ||
| "1121", | ||
| ], | ||
| listRefObj1: ["1121", "1123", "1131", "1133", undefined, null, "1121"], |
There was a problem hiding this comment.
I don't know why exactly these unresolvable references were working previously but not now. I modified this test as minimally as possible to still stress the code that it was (hopefully) intending to stress.
| expect(src[2]).toEqual(deserVid) | ||
| }) | ||
|
|
||
| it.skip("(de)serialize class hierarchy with multiple levels - including classes without serializable properties", () => { |
There was a problem hiding this comment.
This skipped test triggers the issue I discussed in issue #187.
| expect(src[2]).toEqual(deserVid) | ||
| }) | ||
|
|
||
| it("(de)serializes class hierarchy with many forward references", () => { |
There was a problem hiding this comment.
This is the test case that fails without the === 0 change above.
1 participant
Description
There are valid deserialization scenarios where the number of pending references can equal the pending reference count during deserialization. My coworker and I hit one such scenario and I thought I'd track down the issue and contribute back.
I included a test case that stresses my valid deserialization scenario. The scenario is highlighted in my comments in issue #193.
I also added a skipping test to prove out my comments on issue #187.
Fixes #193.
Type of Change
Checklist