Skip to content

Commit 6f76a2a

Browse files
Ryan Holdrenfacebook-github-bot
Ryan Holdren
authored andcommitted
Propegate empty arrays into the store when handling errors on noncompliant lists
Reviewed By: itamark Differential Revision: D65852550 fbshipit-source-id: 46315f62a08f578885c174939840678f9da80ca6
1 parent 79c12a0 commit 6f76a2a

9 files changed

+1006
-11
lines changed

‎packages/relay-runtime/store/RelayReader.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import type {
4949
import type {Arguments} from './RelayStoreUtils';
5050
import type {EvaluationResult, ResolverCache} from './ResolverCache';
5151

52+
const RelayFeatureFlags = require('../util/RelayFeatureFlags');
5253
const {
5354
isSuspenseSentinel,
5455
} = require('./live-resolvers/LiveResolverSuspenseSentinel');
@@ -1091,7 +1092,12 @@ class RelayReader {
10911092
const fieldName = field.alias ?? field.name;
10921093
const storageKey = getStorageKey(field, this._variables);
10931094
const value = RelayModernRecord.getValue(record, storageKey);
1094-
if (value === null) {
1095+
if (
1096+
value === null ||
1097+
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
1098+
Array.isArray(value) &&
1099+
value.length === 0)
1100+
) {
10951101
this._maybeAddErrorResponseFields(record, storageKey);
10961102
} else if (value === undefined) {
10971103
this._markDataAsMissing(fieldName);
@@ -1243,7 +1249,12 @@ class RelayReader {
12431249
): ?mixed {
12441250
const storageKey = getStorageKey(field, this._variables);
12451251
const linkedIDs = RelayModernRecord.getLinkedRecordIDs(record, storageKey);
1246-
if (linkedIDs === null) {
1252+
if (
1253+
linkedIDs === null ||
1254+
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
1255+
Array.isArray(linkedIDs) &&
1256+
linkedIDs.length === 0)
1257+
) {
12471258
this._maybeAddErrorResponseFields(record, storageKey);
12481259
}
12491260
return this._readLinkedIds(field, linkedIDs, record, data);

‎packages/relay-runtime/store/RelayResponseNormalizer.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,11 @@ class RelayResponseNormalizer {
472472
const responseKey = selection.alias || selection.name;
473473
const storageKey = getStorageKey(selection, this._variables);
474474
const fieldValue = data[responseKey];
475-
if (
476-
fieldValue == null ||
477-
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
478-
Array.isArray(fieldValue) &&
479-
fieldValue.length === 0)
480-
) {
475+
const isNoncompliantlyNullish =
476+
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
477+
Array.isArray(fieldValue) &&
478+
fieldValue.length === 0;
479+
if (fieldValue == null || isNoncompliantlyNullish) {
481480
if (fieldValue === undefined) {
482481
// Fields may be missing in the response in two main cases:
483482
// - Inside a client extension: the server will not generally return
@@ -524,7 +523,16 @@ class RelayResponseNormalizer {
524523
);
525524
}
526525
}
527-
RelayModernRecord.setValue(record, storageKey, null);
526+
if (isNoncompliantlyNullish) {
527+
// We need to preserve the fact that we received an empty list
528+
if (selection.kind === 'LinkedField') {
529+
RelayModernRecord.setLinkedRecordIDs(record, storageKey, []);
530+
} else {
531+
RelayModernRecord.setValue(record, storageKey, []);
532+
}
533+
} else {
534+
RelayModernRecord.setValue(record, storageKey, null);
535+
}
528536
const errorTrie = this._errorTrie;
529537
if (errorTrie != null) {
530538
const errors = getErrorsByKey(errorTrie, responseKey);

‎packages/relay-runtime/store/__tests__/RelayReader-RelayErrorHandling-test.js

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
'use strict';
1212

1313
const {graphql} = require('../../query/GraphQLTag');
14+
const RelayFeatureFlags = require('../../util/RelayFeatureFlags');
1415
const {
1516
createOperationDescriptor,
1617
} = require('../RelayModernOperationDescriptor');
@@ -635,4 +636,251 @@ describe('RelayReader error fields', () => {
635636
},
636637
]);
637638
});
639+
640+
let wasNoncompliantErrorHandlingOnListsEnabled;
641+
642+
beforeEach(() => {
643+
wasNoncompliantErrorHandlingOnListsEnabled =
644+
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS;
645+
});
646+
647+
afterEach(() => {
648+
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS =
649+
wasNoncompliantErrorHandlingOnListsEnabled;
650+
});
651+
652+
describe('when noncompliant error handling on lists is enabled', () => {
653+
beforeEach(() => {
654+
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS = true;
655+
});
656+
657+
describe('when query has @throwOnFieldError directive', () => {
658+
it('has errors that will throw when the linked field is an empty list', () => {
659+
const source = RelayRecordSource.create({
660+
'1': {
661+
__id: '1',
662+
__typename: 'User',
663+
'friends(first:3)': {
664+
__ref: 'client:1:friends(first:3)',
665+
},
666+
id: '1',
667+
},
668+
'client:1:friends(first:3)': {
669+
__id: 'client:1:friends(first:3)',
670+
__typename: 'FriendsConnection',
671+
__errors: {
672+
edges: [
673+
{
674+
message: 'There was an error!',
675+
},
676+
],
677+
},
678+
edges: {
679+
__refs: [],
680+
},
681+
},
682+
'client:root': {
683+
__id: 'client:root',
684+
__typename: '__Root',
685+
'node(id:"1")': {__ref: '1'},
686+
},
687+
});
688+
689+
const FooQuery = graphql`
690+
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithThrowOnFieldErrorQuery
691+
@throwOnFieldError {
692+
node(id: "1") {
693+
id
694+
__typename
695+
... on User {
696+
friends(first: 3) {
697+
edges {
698+
cursor
699+
}
700+
}
701+
}
702+
}
703+
}
704+
`;
705+
const operation = createOperationDescriptor(FooQuery, {});
706+
const {errorResponseFields} = read(source, operation.fragment);
707+
708+
expect(errorResponseFields).toEqual([
709+
{
710+
fieldPath: '',
711+
handled: false,
712+
error: {message: 'There was an error!'},
713+
kind: 'relay_field_payload.error',
714+
owner:
715+
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithThrowOnFieldErrorQuery',
716+
shouldThrow: true,
717+
},
718+
]);
719+
});
720+
721+
it('has errors that will throw when the scalar field is an empty list', () => {
722+
const source = RelayRecordSource.create({
723+
'1': {
724+
__id: '1',
725+
__typename: 'User',
726+
__errors: {
727+
emailAddresses: [
728+
{
729+
message: 'There was an error!',
730+
},
731+
],
732+
},
733+
id: '1',
734+
emailAddresses: [],
735+
},
736+
'client:root': {
737+
__id: 'client:root',
738+
__typename: '__Root',
739+
'node(id:"1")': {__ref: '1'},
740+
},
741+
});
742+
743+
const FooQuery = graphql`
744+
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithThrowOnFieldErrorQuery
745+
@throwOnFieldError {
746+
node(id: "1") {
747+
id
748+
__typename
749+
... on User {
750+
emailAddresses
751+
}
752+
}
753+
}
754+
`;
755+
const operation = createOperationDescriptor(FooQuery, {});
756+
const {errorResponseFields} = read(source, operation.fragment);
757+
758+
expect(errorResponseFields).toEqual([
759+
{
760+
fieldPath: '',
761+
handled: false,
762+
error: {message: 'There was an error!'},
763+
kind: 'relay_field_payload.error',
764+
owner:
765+
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithThrowOnFieldErrorQuery',
766+
shouldThrow: true,
767+
},
768+
]);
769+
});
770+
});
771+
772+
describe('when query does not have the @throwOnFieldError directive', () => {
773+
it('has errors that wont throw when the linked field is an empty list', () => {
774+
const source = RelayRecordSource.create({
775+
'1': {
776+
__id: '1',
777+
__typename: 'User',
778+
'friends(first:3)': {
779+
__ref: 'client:1:friends(first:3)',
780+
},
781+
id: '1',
782+
},
783+
'client:1:friends(first:3)': {
784+
__id: 'client:1:friends(first:3)',
785+
__typename: 'FriendsConnection',
786+
__errors: {
787+
edges: [
788+
{
789+
message: 'There was an error!',
790+
},
791+
],
792+
},
793+
edges: {
794+
__refs: [],
795+
},
796+
},
797+
'client:root': {
798+
__id: 'client:root',
799+
__typename: '__Root',
800+
'node(id:"1")': {__ref: '1'},
801+
},
802+
});
803+
804+
const FooQuery = graphql`
805+
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithoutThrowOnFieldErrorQuery {
806+
node(id: "1") {
807+
id
808+
__typename
809+
... on User {
810+
friends(first: 3) {
811+
edges {
812+
cursor
813+
}
814+
}
815+
}
816+
}
817+
}
818+
`;
819+
const operation = createOperationDescriptor(FooQuery, {});
820+
const {data, errorResponseFields} = read(source, operation.fragment);
821+
822+
expect(data.node.friends.edges).toEqual([]);
823+
expect(errorResponseFields).toEqual([
824+
{
825+
fieldPath: '',
826+
handled: false,
827+
error: {message: 'There was an error!'},
828+
kind: 'relay_field_payload.error',
829+
owner:
830+
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithoutThrowOnFieldErrorQuery',
831+
shouldThrow: false,
832+
},
833+
]);
834+
});
835+
836+
it('has errors that wont throw when the scalar field is an empty list', () => {
837+
const source = RelayRecordSource.create({
838+
'1': {
839+
__id: '1',
840+
__typename: 'User',
841+
__errors: {
842+
emailAddresses: [
843+
{
844+
message: 'There was an error!',
845+
},
846+
],
847+
},
848+
id: '1',
849+
emailAddresses: [],
850+
},
851+
'client:root': {
852+
__id: 'client:root',
853+
__typename: '__Root',
854+
'node(id:"1")': {__ref: '1'},
855+
},
856+
});
857+
858+
const FooQuery = graphql`
859+
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithoutThrowOnFieldErrorQuery {
860+
node(id: "1") {
861+
id
862+
__typename
863+
... on User {
864+
emailAddresses
865+
}
866+
}
867+
}
868+
`;
869+
const operation = createOperationDescriptor(FooQuery, {});
870+
const {data, errorResponseFields} = read(source, operation.fragment);
871+
expect(data.node.emailAddresses).toEqual([]);
872+
expect(errorResponseFields).toEqual([
873+
{
874+
fieldPath: '',
875+
handled: false,
876+
error: {message: 'There was an error!'},
877+
kind: 'relay_field_payload.error',
878+
owner:
879+
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithoutThrowOnFieldErrorQuery',
880+
shouldThrow: false,
881+
},
882+
]);
883+
});
884+
});
885+
});
638886
});

0 commit comments

Comments
 (0)