Skip to content

Commit

Permalink
Relay: Add parentType to Refragmentable id Fields
Browse files Browse the repository at this point in the history
Summary: Anytime a generated field has the possibility of being refragmented (i.e. splitting the fields of dynamically-typed nodes into typed fragments), we need to make sure we include the `parentType`. Otherwise, if the generated field is refragmented, `getParentType()` will throw.

This fixes the two places where this can currently happen:

 - `RelayQueryPath` generates `id` fields for the query path.
 - `diffRelayQuery` generates `id` fields for `connection(find: ...){edges{node{id}}}`.

@​bypass-lint

Reviewed By: @josephsavona

Differential Revision: D2432010
  • Loading branch information
yungsters authored and facebook-github-bot-0 committed Sep 10, 2015
1 parent dedefdd commit 2f1e287
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/query/RelayQueryPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,13 @@ class RelayQueryPath {
if (GraphQLStoreDataHandler.isClientID(dataID)) {
return new RelayQueryPath(node, this);
} else {
// refetchable ID
var idField = RelayQuery.Node.buildField('id', null, null, {
parentType: RelayNodeInterface.NODE_TYPE,
});
var root = RelayQuery.Node.buildRoot(
RelayNodeInterface.NODE,
dataID,
[RelayQuery.Node.buildField('id')],
[idField],
{rootArg: RelayNodeInterface.ID},
this.getName()
);
Expand Down
12 changes: 10 additions & 2 deletions src/query/__tests__/RelayQueryPath-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var RelayTestUtils = require('RelayTestUtils');
RelayTestUtils.unmockRelay();

var Relay = require('Relay');
var RelayNodeInterface = require('RelayNodeInterface');
var RelayQueryPath = require('RelayQueryPath');

describe('RelayQueryPath', () => {
Expand Down Expand Up @@ -53,16 +54,23 @@ describe('RelayQueryPath', () => {
name
}
`;

var path = new RelayQueryPath(query);
expect(path.getQuery(getNode(fragment))).toEqualQueryRoot(getNode(Relay.QL`
expect(path.getName()).toBe(query.getName());

var pathQuery = path.getQuery(getNode(fragment));
expect(pathQuery).toEqualQueryRoot(getNode(Relay.QL`
query {
node(id:"123") {
id,
${fragment},
}
}
`));
expect(path.getName()).toBe(query.getName());

// Ensure that the generated `id` field contains necessary metadata.
var idField = pathQuery.getFieldByStorageKey('id');
expect(idField.getParentType()).toBe(RelayNodeInterface.NODE_TYPE);
});

it('creates root paths for argument-less root calls with IDs', () => {
Expand Down
15 changes: 13 additions & 2 deletions src/traversal/__tests__/diffRelayQuery_connection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jest

var Relay = require('Relay');
var RelayConnectionInterface = require('RelayConnectionInterface');
var RelayNodeInterface = require('RelayNodeInterface');
var RelayQueryTracker = require('RelayQueryTracker');
var diffRelayQuery = require('diffRelayQuery');
var generateRQLFieldAlias = require('generateRQLFieldAlias');
Expand Down Expand Up @@ -320,7 +321,8 @@ describe('diffRelayQuery', () => {
`));
});

it('fetches missing `node` data via a `node()` query and missing `edges` data via a `connection.find()` query if connection is findable', () => {
it('fetches missing `node` data via a `node()` query and missing `edges` ' +
'data via a `connection.find()` query if connection is findable', () => {
var records = {};
var store = new RelayRecordStore({records}, {map: rootCallMap});
var tracker = new RelayQueryTracker();
Expand Down Expand Up @@ -450,9 +452,18 @@ describe('diffRelayQuery', () => {
}
}
`));

// Ensure that the generated `id` field contains necessary metadata.
var idField = diffQueries[5]
.getFieldByStorageKey('newsFeed')
.getFieldByStorageKey('edges')
.getFieldByStorageKey('node')
.getFieldByStorageKey('id');
expect(idField.getParentType()).toBe(RelayNodeInterface.NODE_TYPE);
});

it('fetches missing `node` data via a `node()` query and warns about unfetchable `edges` data if connection is not findable', () => {
it('fetches missing `node` data via a `node()` query and warns about ' +
'unfetchable `edges` data if connection is not findable', () => {
var records = {};
var store = new RelayRecordStore({records}, {map: rootCallMap});
var tracker = new RelayQueryTracker();
Expand Down
6 changes: 5 additions & 1 deletion src/traversal/diffRelayQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ var invariant = require('invariant');
var warning = require('warning');

var {EDGES, NODE, PAGE_INFO} = RelayConnectionInterface;
var idField = RelayQuery.Node.buildField('id', null, null, {
parentType: RelayNodeInterface.NODE_TYPE,
requisite: true,
});
var nodeWithID = RelayQuery.Node.buildField(
RelayNodeInterface.NODE,
null,
[RelayQuery.Node.buildField('id', null, null, {requisite: true})],
[idField],
);

import type {DataID} from 'RelayInternalTypes';
Expand Down

0 comments on commit 2f1e287

Please sign in to comment.