Skip to content

Commit

Permalink
Propegate empty arrays into the store when handling errors on noncomp…
Browse files Browse the repository at this point in the history 
…liant lists

Reviewed By: itamark

Differential Revision: D65852550

fbshipit-source-id: 46315f62a08f578885c174939840678f9da80ca6
  • Loading branch information
@facebook-github-bot
Ryan Holdren authored and facebook-github-bot committed Nov 22, 2024
1 parent 79c12a0 commit 6f76a2a
Show file tree
Hide file tree
Showing 9 changed files with 1,006 additions and 11 deletions.
15 changes: 13 additions & 2 deletions packages/relay-runtime/store/RelayReader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import type {
import type {Arguments} from './RelayStoreUtils';
import type {EvaluationResult, ResolverCache} from './ResolverCache';

const RelayFeatureFlags = require('../util/RelayFeatureFlags');
const {
isSuspenseSentinel,
} = require('./live-resolvers/LiveResolverSuspenseSentinel');
Expand Down Expand Up @@ -1091,7 +1092,12 @@ class RelayReader {
const fieldName = field.alias ?? field.name;
const storageKey = getStorageKey(field, this._variables);
const value = RelayModernRecord.getValue(record, storageKey);
if (value === null) {
if (
value === null ||
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
Array.isArray(value) &&
value.length === 0)
) {
this._maybeAddErrorResponseFields(record, storageKey);
} else if (value === undefined) {
this._markDataAsMissing(fieldName);
Expand Down Expand Up @@ -1243,7 +1249,12 @@ class RelayReader {
): ?mixed {
const storageKey = getStorageKey(field, this._variables);
const linkedIDs = RelayModernRecord.getLinkedRecordIDs(record, storageKey);
if (linkedIDs === null) {
if (
linkedIDs === null ||
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
Array.isArray(linkedIDs) &&
linkedIDs.length === 0)
) {
this._maybeAddErrorResponseFields(record, storageKey);
}
return this._readLinkedIds(field, linkedIDs, record, data);
Expand Down
22 changes: 15 additions & 7 deletions packages/relay-runtime/store/RelayResponseNormalizer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,11 @@ class RelayResponseNormalizer {
const responseKey = selection.alias || selection.name;
const storageKey = getStorageKey(selection, this._variables);
const fieldValue = data[responseKey];
if (
fieldValue == null ||
(RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
Array.isArray(fieldValue) &&
fieldValue.length === 0)
) {
const isNoncompliantlyNullish =
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS &&
Array.isArray(fieldValue) &&
fieldValue.length === 0;
if (fieldValue == null || isNoncompliantlyNullish) {
if (fieldValue === undefined) {
// Fields may be missing in the response in two main cases:
// - Inside a client extension: the server will not generally return
Expand Down Expand Up @@ -524,7 +523,16 @@ class RelayResponseNormalizer {
);
}
}
RelayModernRecord.setValue(record, storageKey, null);
if (isNoncompliantlyNullish) {
// We need to preserve the fact that we received an empty list
if (selection.kind === 'LinkedField') {
RelayModernRecord.setLinkedRecordIDs(record, storageKey, []);
} else {
RelayModernRecord.setValue(record, storageKey, []);
}
} else {
RelayModernRecord.setValue(record, storageKey, null);
}
const errorTrie = this._errorTrie;
if (errorTrie != null) {
const errors = getErrorsByKey(errorTrie, responseKey);
Expand Down
248 changes: 248 additions & 0 deletions packages/relay-runtime/store/__tests__/RelayReader-RelayErrorHandling-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

const {graphql} = require('../../query/GraphQLTag');
const RelayFeatureFlags = require('../../util/RelayFeatureFlags');
const {
createOperationDescriptor,
} = require('../RelayModernOperationDescriptor');
Expand Down Expand Up @@ -635,4 +636,251 @@ describe('RelayReader error fields', () => {
},
]);
});

let wasNoncompliantErrorHandlingOnListsEnabled;

beforeEach(() => {
wasNoncompliantErrorHandlingOnListsEnabled =
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS;
});

afterEach(() => {
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS =
wasNoncompliantErrorHandlingOnListsEnabled;
});

describe('when noncompliant error handling on lists is enabled', () => {
beforeEach(() => {
RelayFeatureFlags.ENABLE_NONCOMPLIANT_ERROR_HANDLING_ON_LISTS = true;
});

describe('when query has @throwOnFieldError directive', () => {
it('has errors that will throw when the linked field is an empty list', () => {
const source = RelayRecordSource.create({
'1': {
__id: '1',
__typename: 'User',
'friends(first:3)': {
__ref: 'client:1:friends(first:3)',
},
id: '1',
},
'client:1:friends(first:3)': {
__id: 'client:1:friends(first:3)',
__typename: 'FriendsConnection',
__errors: {
edges: [
{
message: 'There was an error!',
},
],
},
edges: {
__refs: [],
},
},
'client:root': {
__id: 'client:root',
__typename: '__Root',
'node(id:"1")': {__ref: '1'},
},
});

const FooQuery = graphql`
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithThrowOnFieldErrorQuery
@throwOnFieldError {
node(id: "1") {
id
__typename
... on User {
friends(first: 3) {
edges {
cursor
}
}
}
}
}
`;
const operation = createOperationDescriptor(FooQuery, {});
const {errorResponseFields} = read(source, operation.fragment);

expect(errorResponseFields).toEqual([
{
fieldPath: '',
handled: false,
error: {message: 'There was an error!'},
kind: 'relay_field_payload.error',
owner:
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithThrowOnFieldErrorQuery',
shouldThrow: true,
},
]);
});

it('has errors that will throw when the scalar field is an empty list', () => {
const source = RelayRecordSource.create({
'1': {
__id: '1',
__typename: 'User',
__errors: {
emailAddresses: [
{
message: 'There was an error!',
},
],
},
id: '1',
emailAddresses: [],
},
'client:root': {
__id: 'client:root',
__typename: '__Root',
'node(id:"1")': {__ref: '1'},
},
});

const FooQuery = graphql`
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithThrowOnFieldErrorQuery
@throwOnFieldError {
node(id: "1") {
id
__typename
... on User {
emailAddresses
}
}
}
`;
const operation = createOperationDescriptor(FooQuery, {});
const {errorResponseFields} = read(source, operation.fragment);

expect(errorResponseFields).toEqual([
{
fieldPath: '',
handled: false,
error: {message: 'There was an error!'},
kind: 'relay_field_payload.error',
owner:
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithThrowOnFieldErrorQuery',
shouldThrow: true,
},
]);
});
});

describe('when query does not have the @throwOnFieldError directive', () => {
it('has errors that wont throw when the linked field is an empty list', () => {
const source = RelayRecordSource.create({
'1': {
__id: '1',
__typename: 'User',
'friends(first:3)': {
__ref: 'client:1:friends(first:3)',
},
id: '1',
},
'client:1:friends(first:3)': {
__id: 'client:1:friends(first:3)',
__typename: 'FriendsConnection',
__errors: {
edges: [
{
message: 'There was an error!',
},
],
},
edges: {
__refs: [],
},
},
'client:root': {
__id: 'client:root',
__typename: '__Root',
'node(id:"1")': {__ref: '1'},
},
});

const FooQuery = graphql`
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithoutThrowOnFieldErrorQuery {
node(id: "1") {
id
__typename
... on User {
friends(first: 3) {
edges {
cursor
}
}
}
}
}
`;
const operation = createOperationDescriptor(FooQuery, {});
const {data, errorResponseFields} = read(source, operation.fragment);

expect(data.node.friends.edges).toEqual([]);
expect(errorResponseFields).toEqual([
{
fieldPath: '',
handled: false,
error: {message: 'There was an error!'},
kind: 'relay_field_payload.error',
owner:
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyLinkedFieldWithoutThrowOnFieldErrorQuery',
shouldThrow: false,
},
]);
});

it('has errors that wont throw when the scalar field is an empty list', () => {
const source = RelayRecordSource.create({
'1': {
__id: '1',
__typename: 'User',
__errors: {
emailAddresses: [
{
message: 'There was an error!',
},
],
},
id: '1',
emailAddresses: [],
},
'client:root': {
__id: 'client:root',
__typename: '__Root',
'node(id:"1")': {__ref: '1'},
},
});

const FooQuery = graphql`
query RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithoutThrowOnFieldErrorQuery {
node(id: "1") {
id
__typename
... on User {
emailAddresses
}
}
}
`;
const operation = createOperationDescriptor(FooQuery, {});
const {data, errorResponseFields} = read(source, operation.fragment);
expect(data.node.emailAddresses).toEqual([]);
expect(errorResponseFields).toEqual([
{
fieldPath: '',
handled: false,
error: {message: 'There was an error!'},
kind: 'relay_field_payload.error',
owner:
'RelayReaderRelayErrorHandlingTestNoncompliantEmptyScalarFieldWithoutThrowOnFieldErrorQuery',
shouldThrow: false,
},
]);
});
});
});
});
Loading

0 comments on commit 6f76a2a

Please sign in to comment.