Skip to content

Commit

Permalink
Improve edge resolver error messages
Browse files Browse the repository at this point in the history 
Differential Revision: D62516826

fbshipit-source-id: 0bec759c1a16f64f5e18c2f52e3190fdf42d97eb
  • Loading branch information
@gordyf @facebook-github-bot
gordyf authored and facebook-github-bot committed Sep 12, 2024
1 parent f9365f7 commit 852fbc3
Show file tree
Hide file tree
Showing 6 changed files with 751 additions and 25 deletions.
69 changes: 50 additions & 19 deletions packages/relay-runtime/store/RelayReader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -840,22 +840,31 @@ class RelayReader {
if (field.linkedField.plural) {
invariant(
Array.isArray(clientEdgeResolverResponse),
'Expected plural Client Edge Relay Resolver to return an array containing IDs or objects with shape {id}.',
'Expected plural Client Edge Relay Resolver at `%s` in `%s` to return an array containing IDs or objects with shape {id}.',
backingField.path,
this._owner.identifier,
);
let storeIDs: $ReadOnlyArray<DataID>;
invariant(
field.kind === 'ClientEdgeToClientObject',
'Unexpected Client Edge to plural server type. This should be prevented by the compiler.',
'Unexpected Client Edge to plural server type `%s`. This should be prevented by the compiler.',
field.kind,
);
if (field.backingField.normalizationInfo == null) {
// @edgeTo case where we need to ensure that the record has `id` field
storeIDs = clientEdgeResolverResponse.map(itemResponse => {
const concreteType = field.concreteType ?? itemResponse.__typename;
invariant(
typeof concreteType === 'string',
'Expected resolver modeling an edge to an abstract type to return an object with a `__typename` property.',
'Expected resolver for field at `%s` in `%s` modeling an edge to an abstract type to return an object with a `__typename` property.',
backingField.path,
this._owner.identifier,
);
const localId = extractIdFromResponse(
itemResponse,
backingField.path,
this._owner.identifier,
);
const localId = extractIdFromResponse(itemResponse);
const id = this._resolverCache.ensureClientRecord(
localId,
concreteType,
Expand All @@ -866,9 +875,11 @@ class RelayReader {
const modelResolver = modelResolvers[concreteType];
invariant(
modelResolver !== undefined,
`Invalid \`__typename\` returned by resolver. Expected one of ${Object.keys(
modelResolvers,
).join(', ')} but got \`${concreteType}\`.`,
'Invalid `__typename` returned by resolver at `%s` in `%s`. Expected one of %s but got `%s`.',
backingField.path,
this._owner.identifier,
Object.keys(modelResolvers).join(', '),
concreteType,
);
const model = this._readResolverFieldImpl(modelResolver, id);
return model != null ? id : null;
Expand All @@ -877,7 +888,9 @@ class RelayReader {
});
} else {
// The normalization process in LiveResolverCache should take care of generating the correct ID.
storeIDs = clientEdgeResolverResponse.map(extractIdFromResponse);
storeIDs = clientEdgeResolverResponse.map(obj =>
extractIdFromResponse(obj, backingField.path, this._owner.identifier),
);
}
this._clientEdgeTraversalPath.push(null);
const edgeValues = this._readLinkedIds(
Expand All @@ -890,7 +903,11 @@ class RelayReader {
data[fieldName] = edgeValues;
return edgeValues;
} else {
const id = extractIdFromResponse(clientEdgeResolverResponse);
const id = extractIdFromResponse(
clientEdgeResolverResponse,
backingField.path,
this._owner.identifier,
);
let storeID: DataID;
const concreteType =
field.concreteType ?? clientEdgeResolverResponse.__typename;
Expand All @@ -899,7 +916,9 @@ class RelayReader {
if (field.backingField.normalizationInfo == null) {
invariant(
typeof concreteType === 'string',
'Expected resolver modeling an edge to an abstract type to return an object with a `__typename` property.',
'Expected resolver for field at `%s` in `%s` modeling an edge to an abstract type to return an object with a `__typename` property.',
backingField.path,
this._owner.identifier,
);
// @edgeTo case where we need to ensure that the record has `id` field
storeID = this._resolverCache.ensureClientRecord(id, concreteType);
Expand All @@ -921,14 +940,18 @@ class RelayReader {
if (modelResolvers != null) {
invariant(
typeof concreteType === 'string',
'Expected resolver modeling an edge to an abstract type to return an object with a `__typename` property.',
'Expected resolver for field at `%s` in `%s` modeling an edge to an abstract type to return an object with a `__typename` property.',
backingField.path,
this._owner.identifier,
);
const modelResolver = modelResolvers[concreteType];
invariant(
modelResolver !== undefined,
`Invalid \`__typename\` returned by resolver. Expected one of ${Object.keys(
modelResolvers,
).join(', ')} but got \`${concreteType}\`.`,
'Invalid `__typename` returned by resolver at `%s` in `%s`. Expected one of %s but got `%s`.',
backingField.path,
this._owner.identifier,
Object.keys(modelResolvers).join(', '),
concreteType,
);
const model = this._readResolverFieldImpl(modelResolver, storeID);
if (model == null) {
Expand All @@ -943,9 +966,10 @@ class RelayReader {
const prevData = data[fieldName];
invariant(
prevData == null || typeof prevData === 'object',
'RelayReader(): Expected data for field `%s` on record `%s` ' +
'RelayReader(): Expected data for field at `%s` in `%s` on record `%s` ' +
'to be an object, got `%s`.',
fieldName,
backingField.path,
this._owner.identifier,
RelayModernRecord.getDataID(record),
prevData,
);
Expand Down Expand Up @@ -999,9 +1023,10 @@ class RelayReader {
const prevData = data[fieldName];
invariant(
prevData == null || typeof prevData === 'object',
'RelayReader(): Expected data for field `%s` on record `%s` ' +
'RelayReader(): Expected data for field `%s` at `%s` on record `%s` ' +
'to be an object, got `%s`.',
fieldName,
this._owner.identifier,
RelayModernRecord.getDataID(record),
prevData,
);
Expand Down Expand Up @@ -1437,7 +1462,11 @@ function getResolverValue(
return [resolverResult, resolverError];
}

function extractIdFromResponse(individualResponse: mixed): string {
function extractIdFromResponse(
individualResponse: mixed,
path: string,
owner: string,
): string {
if (typeof individualResponse === 'string') {
return individualResponse;
} else if (
Expand All @@ -1449,7 +1478,9 @@ function extractIdFromResponse(individualResponse: mixed): string {
}
invariant(
false,
'Expected object returned from an edge resolver to be a string or an object with an `id` property',
'Expected object returned from edge resolver to be a string or an object with an `id` property at path %s in %s,',
path,
owner,
);
}

Expand Down
58 changes: 52 additions & 6 deletions packages/relay-runtime/store/__tests__/RelayReader-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,15 @@ describe('RelayReader', () => {
}
}
`;
const owner = createOperationDescriptor(UserFriends, {id: '1'});
const {data, seenRecords} = read(
source,
createReaderSelector(UserFriends.fragment, ROOT_ID, {id: '1'}),
createReaderSelector(
UserFriends.fragment,
ROOT_ID,
{id: '1'},
owner.request,
),
);
expect(data).toEqual({
node: {
Expand Down Expand Up @@ -544,6 +550,13 @@ describe('RelayReader', () => {
},
};
source = RelayRecordSource.create(records);
const UserQuery = graphql`
query RelayReaderTestReadsHandleFieldsForFragmentsUserFriendsQuery {
me {
...RelayReaderTestReadsHandleFieldsForFragmentsUserFriends
}
}
`;
const UserFriends = graphql`
fragment RelayReaderTestReadsHandleFieldsForFragmentsUserFriends on User {
friends(first: 1) @__clientField(handle: "bestFriends") {
Expand All @@ -557,9 +570,10 @@ describe('RelayReader', () => {
}
}
`;
const owner = createOperationDescriptor(UserQuery, {});
const {data, seenRecords} = read(
source,
createReaderSelector(UserFriends, '1', {}),
createReaderSelector(UserFriends, '1', {}, owner.request),
);
expect(data).toEqual({
friends: {
Expand Down Expand Up @@ -746,9 +760,10 @@ describe('RelayReader', () => {
},
};
source = RelayRecordSource.create(storeData);
const owner = createOperationDescriptor(BarQuery, {});
const {data, seenRecords, isMissingData} = read(
source,
createReaderSelector(BarFragment, '1', {}),
createReaderSelector(BarFragment, '1', {}, owner.request),
);
expect(data).toEqual({
id: '1',
Expand Down Expand Up @@ -1192,6 +1207,14 @@ describe('RelayReader', () => {
});

it('should have `isMissingData = false` if data is available', () => {
const UserQuery = graphql`
query RelayReaderTestShouldHaveIsmissingdataFalseIfDataIsAvailableUserFriendsQuery {
me {
...RelayReaderTestShouldHaveIsmissingdataFalseIfDataIsAvailableUserFriends
}
}
`;

const UserFriends = graphql`
fragment RelayReaderTestShouldHaveIsmissingdataFalseIfDataIsAvailableUserFriends on User {
id
Expand All @@ -1205,9 +1228,12 @@ describe('RelayReader', () => {
}
}
`;

const owner = createOperationDescriptor(UserQuery, {});

const {data, isMissingData} = read(
source,
createReaderSelector(UserFriends, '1', {}),
createReaderSelector(UserFriends, '1', {}, owner.request),
);
expect(data.friends.edges).toEqual([
{
Expand All @@ -1228,6 +1254,14 @@ describe('RelayReader', () => {
});

it('should have `isMissingData = true` if data is missing in the node', () => {
const UserQuery = graphql`
query RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingInTheNodeUserFriendsQuery {
me {
...RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingInTheNodeUserFriends
}
}
`;

const UserFriends = graphql`
fragment RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingInTheNodeUserFriends on User {
id
Expand All @@ -1242,9 +1276,11 @@ describe('RelayReader', () => {
}
}
`;

const owner = createOperationDescriptor(UserQuery, {});
const {data, isMissingData} = read(
source,
createReaderSelector(UserFriends, '1', {}),
createReaderSelector(UserFriends, '1', {}, owner.request),
);
expect(data.friends.edges).toEqual([
{
Expand All @@ -1265,6 +1301,14 @@ describe('RelayReader', () => {
});

it('should have `isMissingData = true` if data is missing for connection', () => {
const UserQuery = graphql`
query RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingForConnectionUserFriendsQuery {
me {
...RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingForConnectionUserFriends
}
}
`;

const UserFriends = graphql`
fragment RelayReaderTestShouldHaveIsmissingdataTrueIfDataIsMissingForConnectionUserFriends on User {
id
Expand All @@ -1278,9 +1322,11 @@ describe('RelayReader', () => {
}
}
`;

const owner = createOperationDescriptor(UserQuery, {});
const {data, isMissingData} = read(
source,
createReaderSelector(UserFriends, '2', {}),
createReaderSelector(UserFriends, '2', {}, owner.request),
);
expect(data.id).toBe('2');
expect(data.friends.edges).not.toBeDefined();
Expand Down
Loading

0 comments on commit 852fbc3

Please sign in to comment.