Skip to content

Commit

Permalink
Fix recycling of data in FragmentResource::subscribe
Browse files Browse the repository at this point in the history
Reviewed By: kassens

Differential Revision: D19977832

fbshipit-source-id: e078c22c1a077621f5ea20a84353ad68f5e361f4
  • Loading branch information
josephsavona authored and facebook-github-bot committed Feb 19, 2020
1 parent 5d733bc commit 2093478
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 1 deletion.
7 changes: 6 additions & 1 deletion packages/relay-experimental/FragmentResource.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,13 @@ class FragmentResourceImpl {
const renderData = renderedSnapshot.data;
const currentData = currentSnapshot.data;
const updatedData = recycleNodesInto(renderData, currentData);
currentSnapshot = {
data: updatedData,
isMissingData: currentSnapshot.isMissingData,
seenRecords: currentSnapshot.seenRecords,
selector: currentSnapshot.selector,
};
if (updatedData !== renderData) {
currentSnapshot = {...currentSnapshot, data: updatedData};
this._cache.set(cacheKey, getFragmentResult(cacheKey, currentSnapshot));
didMissUpdates = true;
}
Expand Down
36 changes: 36 additions & 0 deletions packages/relay-experimental/__tests__/useFragmentNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,42 @@ it('should update when fragment data changes', () => {
]);
});

it('should preserve object identity when fragment data changes', () => {
renderSingularFragment();
TestRenderer.act(() => jest.runAllImmediates());
expect(renderSpy).toBeCalledTimes(1);
const prevData = renderSpy.mock.calls[0][0];
expect(prevData).toEqual({
id: '1',
name: 'Alice',
profile_picture: null,
...createFragmentRef('1', singularQuery),
});
renderSpy.mockClear();

TestRenderer.act(() => {
environment.commitPayload(singularQuery, {
node: {
__typename: 'User',
id: '1',
// Update name
name: 'Alice in Wonderland',
},
});
});
TestRenderer.act(() => jest.runAllImmediates());
expect(renderSpy).toBeCalledTimes(1);
const nextData = renderSpy.mock.calls[0][0];
expect(nextData).toEqual({
id: '1',
// Assert that name is updated
name: 'Alice in Wonderland',
profile_picture: null,
...createFragmentRef('1', singularQuery),
});
expect(nextData.__fragments).toBe(prevData.__fragments);
});

it('should re-read and resubscribe to fragment when environment changes', () => {
renderSingularFragment();
assertFragmentResults([
Expand Down
44 changes: 44 additions & 0 deletions packages/relay-experimental/__tests__/useLazyLoadQueryNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,50 @@ describe('useLazyLoadQueryNode', () => {
expectToBeRendered(renderFn, data);
});

it('subscribes to query fragment results and preserves object identity', () => {
const instance = render(environment, <Container variables={variables} />);

expect(instance.toJSON()).toEqual('Fallback');
expectToHaveFetched(environment, query);
expect(renderFn).not.toBeCalled();
expect(environment.retain).toHaveBeenCalledTimes(1);

environment.mock.resolve(gqlQuery, {
data: {
node: {
__typename: 'User',
id: variables.id,
name: 'Alice',
},
},
});

ReactTestRenderer.act(() => {
jest.runAllImmediates();
});
expect(renderFn).toBeCalledTimes(1);
const prevData = renderFn.mock.calls[0][0];
expect(prevData.node.name).toBe('Alice');
renderFn.mockClear();
ReactTestRenderer.act(() => {
jest.runAllImmediates();
});

environment.commitUpdate(store => {
const alice = store.get('1');
if (alice != null) {
alice.setValue('ALICE', 'name');
}
});
expect(renderFn).toBeCalledTimes(1);
const nextData = renderFn.mock.calls[0][0];
expect(nextData.node.name).toBe('ALICE');
renderFn.mockClear();

// object identity is preserved for unchanged data such as fragment references
expect(nextData.node.__fragments).toBe(prevData.node.__fragments);
});

it('fetches and renders correctly even if fetched query data still has missing data', () => {
// This scenario might happen if for example we are making selections on
// abstract types which the concrete type doesn't implemenet
Expand Down
12 changes: 12 additions & 0 deletions packages/relay-runtime/util/__tests__/recycleNodesInto-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ describe('recycleNodesInto', () => {
});

describe('objects', () => {
it('recycles empty objects', () => {
const prevData = {};
const nextData = {};
expect(recycleNodesInto(prevData, nextData)).toBe(prevData);
});

it('recycles nested empty objects', () => {
const prevData = {foo: {bar: {}}};
const nextData = {foo: {bar: {}}};
expect(recycleNodesInto(prevData, nextData)).toBe(prevData);
});

it('recycles equal leaf objects', () => {
const prevData = {foo: 1};
const nextData = {foo: 1};
Expand Down

0 comments on commit 2093478

Please sign in to comment.