Skip to content

Commit

Permalink
Support non-string identifying argument values
Browse files Browse the repository at this point in the history 
Summary:Note: this is inspired by and partially based on iamchenxin's work in facebook#767 and facebook#844. Thanks for the head start!

Relay currently assumes that identifying argument values are strings (numbers *sort* of work, but not really). This builds on facebook#894 (which added support to the plugin for parsing/printing literal InputObjects) by allowing identifying arguments to be basically anything - boolean, number, string, or array/object of the the same.

Key changes include:
- Change the `CallValue` type from mixed to an explicit list of the supported types
- Change `forEachRootCallArg` to return both the literal JS value of the argument as well as a serialized key
- Change all callers of `forEachRootCallArg` (and some places that manually inspected the identifying arg) to correctly choose between the identifying argument value (i.e. when constructing a query with it) or the identifying argument key (for use with `RelayRecordStore.{putDataID,getDataID}`).
- Added tests that the writer correctly creates root records with numeric/object identifying argument values.

Closes facebook#895

Reviewed By: yungsters

Differential Revision: D3003201

Pulled By: josephsavona

fb-gh-sync-id: 43ffbbd37e8d2bd8c0abe2cb792ad8959efb7a42
shipit-source-id: 43ffbbd37e8d2bd8c0abe2cb792ad8959efb7a42
  • Loading branch information
@josephsavona @venepe
josephsavona authored and venepe committed Mar 7, 2016
1 parent 7f5c681 commit 43552ef
Show file tree
Hide file tree
Showing 21 changed files with 510 additions and 94 deletions.
25 changes: 23 additions & 2 deletions scripts/jest/testschema.graphql
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,38 @@
type Root {
checkinSearchQuery(query: CheckinSearchInput): CheckinSearchResult
defaultSettings: Settings,
settings(environment: Environment): Settings
route(waypoints: [WayPoint!]!): Route
me: User
story: Story
node(id: ID): Node
nodes(ids: [ID!]): [Node]
settings(environment: Environment): Settings
story: Story
task(number: Int): Task
username(name: String!): Actor
usernames(names: [String!]!): [Actor]
viewer: Viewer
_mutation: Mutation
}

type Task {
title: String
}

input WayPoint {
lat: String
lon: String
}

type Route {
steps: [RouteStep]
}

type RouteStep {
lat: String
lon: String
note: String
}

type Mutation {
actorSubscribe(input: ActorSubscribeInput): ActorSubscribeResponsePayload
applicationRequestDeleteAll(input: ApplicationRequestDeleteAllInput): ApplicationRequestDeleteAllResponsePayload
Expand Down
221 changes: 203 additions & 18 deletions scripts/jest/testschema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,34 @@
"deprecationReason": null
},
{
"name": "settings",
"name": "route",
"description": null,
"args": [
{
"name": "environment",
"name": "waypoints",
"description": null,
"type": {
"kind": "ENUM",
"name": "Environment",
"ofType": null
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "INPUT_OBJECT",
"name": "WayPoint"
}
}
}
},
"defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "Settings",
"name": "Route",
"ofType": null
},
"isDeprecated": false,
Expand All @@ -86,18 +97,6 @@
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "story",
"description": null,
"args": [],
"type": {
"kind": "OBJECT",
"name": "Story",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "node",
"description": null,
Expand Down Expand Up @@ -156,6 +155,64 @@
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "settings",
"description": null,
"args": [
{
"name": "environment",
"description": null,
"type": {
"kind": "ENUM",
"name": "Environment",
"ofType": null
},
"defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "Settings",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "story",
"description": null,
"args": [],
"type": {
"kind": "OBJECT",
"name": "Story",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "task",
"description": null,
"args": [
{
"name": "number",
"description": null,
"type": {
"kind": "SCALAR",
"name": "Int",
"ofType": null
},
"defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "Task",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "username",
"description": null,
Expand Down Expand Up @@ -384,6 +441,111 @@
],
"possibleTypes": null
},
{
"kind": "INPUT_OBJECT",
"name": "WayPoint",
"description": null,
"fields": null,
"inputFields": [
{
"name": "lat",
"description": null,
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "lon",
"description": null,
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
}
],
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "Route",
"description": null,
"fields": [
{
"name": "steps",
"description": null,
"args": [],
"type": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "OBJECT",
"name": "RouteStep",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "RouteStep",
"description": null,
"fields": [
{
"name": "lat",
"description": null,
"args": [],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "lon",
"description": null,
"args": [],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "note",
"description": null,
"args": [],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "User",
Expand Down Expand Up @@ -6739,6 +6901,29 @@
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "Task",
"description": null,
"fields": [
{
"name": "title",
"description": null,
"args": [],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "Viewer",
Expand Down
4 changes: 3 additions & 1 deletion src/container/__tests__/RelayRenderer_renderArgs-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const Relay = require('Relay');
const RelayQueryConfig = require('RelayQueryConfig');
const RelayRenderer = require('RelayRenderer');
const RelayStore = require('RelayStore');
const RelayTestUtils = require('RelayTestUtils');

describe('RelayRenderer.renderArgs', () => {
let MockComponent;
Expand Down Expand Up @@ -50,6 +51,7 @@ describe('RelayRenderer.renderArgs', () => {
/>,
container
);
jasmine.addMatchers(RelayTestUtils.matchers);
jasmine.addMatchers({
toRenderWithArgs() {
return {
Expand Down Expand Up @@ -202,7 +204,7 @@ describe('RelayRenderer.renderArgs', () => {

const {retry} = render.mock.calls[1][0];
expect(typeof retry).toBe('function');
expect(() => retry()).toThrowError(
expect(() => retry()).toFailInvariant(
'RelayRenderer: You tried to call `retry`, but the last request did ' +
'not fail. You can only call this when the last request has failed.'
);
Expand Down
8 changes: 4 additions & 4 deletions src/interface/RelayOSSNodeInterface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type PayloadResult = {

type RootCallInfo = {
storageKey: string;
identifyingArgValue: ?string;
identifyingArgKey: ?string;
}

/**
Expand Down Expand Up @@ -80,9 +80,9 @@ var RelayOSSNodeInterface = {
var records = getPayloadRecords(query, payload);
var ii = 0;
const storageKey = query.getStorageKey();
forEachRootCallArg(query, identifyingArgValue => {
forEachRootCallArg(query, ({identifyingArgKey}) => {
var result = records[ii++];
var dataID = store.getDataID(storageKey, identifyingArgValue);
var dataID = store.getDataID(storageKey, identifyingArgKey);
if (dataID == null) {
var payloadID = typeof result === 'object' && result ?
result[RelayOSSNodeInterface.ID] :
Expand All @@ -96,7 +96,7 @@ var RelayOSSNodeInterface = {
results.push({
dataID,
result,
rootCallInfo: {storageKey, identifyingArgValue},
rootCallInfo: {storageKey, identifyingArgKey},
});
});
}
Expand Down
6 changes: 3 additions & 3 deletions src/mutation/__tests__/RelayMutationQueue-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ describe('RelayMutationQueue', () => {
it('throws if commit is called more than once', () => {
var transaction = mutationQueue.createTransaction(mockMutation1);
transaction.commit();
expect(() => transaction.commit()).toThrowError(
expect(() => transaction.commit()).toFailInvariant(
'RelayMutationTransaction: Only transactions with status ' +
'`UNCOMMITTED` can be comitted.'
);
Expand Down Expand Up @@ -321,7 +321,7 @@ describe('RelayMutationQueue', () => {

expect(failureCallback1).toBeCalled();
expect(failureCallback2).toBeCalled();
expect(() => transaction1.getStatus()).toThrowError(
expect(() => transaction1.getStatus()).toFailInvariant(
'RelayMutationQueue: `0` is not a valid pending transaction ID.'
);
expect(transaction2.getStatus()).toBe(
Expand Down Expand Up @@ -426,7 +426,7 @@ describe('RelayMutationQueue', () => {
expect(transaction1.getStatus()).toBe(
RelayMutationTransactionStatus.COMMIT_FAILED
);
expect(() => transaction2.getStatus()).toThrowError(
expect(() => transaction2.getStatus()).toFailInvariant(
'RelayMutationQueue: `1` is not a valid pending transaction ID.'
);
expect(transaction3.getStatus()).toBe(
Expand Down
Loading

0 comments on commit 43552ef

Please sign in to comment.