diff --git a/src/container/RelayContainer.js b/src/container/RelayContainer.js index d09bcb7d6e294..623937c06fa71 100644 --- a/src/container/RelayContainer.js +++ b/src/container/RelayContainer.js @@ -916,7 +916,7 @@ function buildContainerFragment( ): GraphQL.Fragment { var fragment = buildRQL.Fragment( fragmentBuilder, - Object.keys(variables) + variables ); invariant( fragment, @@ -968,6 +968,8 @@ function create( adapter: ContainerConstructor.getFragmentNames, }); ContainerConstructor.hasFragment = fragmentName => !!fragments[fragmentName]; + ContainerConstructor.hasVariable = variableName => + Object.prototype.hasOwnProperty.call(initialVariables, variableName); /** * Retrieves a reference to the fragment by name. An optional second argument diff --git a/src/container/__tests__/getRelayQueries-test.js b/src/container/__tests__/getRelayQueries-test.js index 32acecd448541..9500edd16deea 100644 --- a/src/container/__tests__/getRelayQueries-test.js +++ b/src/container/__tests__/getRelayQueries-test.js @@ -35,7 +35,7 @@ describe('getRelayQueries', () => { render() { return
; } - } + }; MockPageContainer = Relay.createContainer(MockPageComponent, { fragments: { @@ -155,15 +155,40 @@ describe('getRelayQueries', () => { ]).toBeWarnedNTimes(1); }); - it('includes route parameters when building component fragment', () => { - var MockRoute = makeRoute(); - var params = {id: '123'}; - var route = new MockRoute(params); - MockPageContainer.getFragment = jest.genMockFunction(); + it('sets root fragment variables to route params', () => { + class MockRoute extends Relay.Route {} + MockRoute.routeName = 'MockRoute'; + MockRoute.path = '/'; + MockRoute.paramDefinitions = {}; + MockRoute.queries = { + first: () => Relay.QL` + query { + viewer + } + `, + }; + + var route = new MockRoute({ + fragmentParam: 'foo', + otherParam: 'bar', + }); + + var AnotherMockContainer = Relay.createContainer(MockPageComponent, { + initialVariables: { + fragmentParam: null, + }, + fragments: { + first: () => Relay.QL`fragment on Node{id}`, + }, + }); - getRelayQueries(MockPageContainer, route); + var queries = getRelayQueries(AnotherMockContainer, route); - expect(MockPageContainer.getFragment.mock.calls.length).toBe(4); - expect(MockPageContainer.getFragment.mock.calls[0][1]).toBe(params); + expect(queries.first.getVariables()).toEqual(route.params); + // `otherParam` is not passed to the root fragment since the variable + // is not defined in the component's `initialVariables`. + expect(queries.first.getChildren()[0].getVariables()).toEqual({ + fragmentParam: 'foo', + }); }); }); diff --git a/src/container/getRelayQueries.js b/src/container/getRelayQueries.js index 9336d6df197b4..44474568d55e7 100644 --- a/src/container/getRelayQueries.js +++ b/src/container/getRelayQueries.js @@ -68,8 +68,8 @@ function getRelayQueries( var concreteQuery = buildRQL.Query( queryBuilder, Component, - Object.keys(route.params), - Component.getFragment(queryName, route.params) + queryName, + route.params ); invariant( concreteQuery !== undefined, diff --git a/src/mutation/RelayMutation.js b/src/mutation/RelayMutation.js index cd599e3d507c4..c6a54861434bc 100644 --- a/src/mutation/RelayMutation.js +++ b/src/mutation/RelayMutation.js @@ -377,7 +377,7 @@ function buildMutationFragment( ): GraphQL.Fragment { var fragment = buildRQL.Fragment( fragmentBuilder, - Object.keys(variables) + variables ); invariant( fragment, diff --git a/src/query/RelayFragmentReference.js b/src/query/RelayFragmentReference.js index 0aca762a8f8c0..307a90b1db324 100644 --- a/src/query/RelayFragmentReference.js +++ b/src/query/RelayFragmentReference.js @@ -18,8 +18,8 @@ import type RelayMetaRoute from 'RelayMetaRoute'; import type {Variables} from 'RelayTypes'; var forEachObject = require('forEachObject'); -var getWeakIdForObject = require('getWeakIdForObject'); var invariant = require('invariant'); +var warning = require('warning'); type Condition = (variables: Variables) => boolean; type FragmentGetter = () => GraphQL.QueryFragment; @@ -199,7 +199,15 @@ class RelayFragmentReference { if (GraphQL.isCallVariable(value)) { value = variables[value.callVariableName]; } - if (value !== undefined) { + if (value === undefined) { + warning( + false, + 'RelayFragmentReference: Variable `%s` is undefined in fragment ' + + '`%s`.', + name, + this._getFragment().name + ); + } else { innerVariables[name] = value; } }); @@ -213,10 +221,6 @@ class RelayFragmentReference { return innerVariables; } - getFragmentName(): string { - return getWeakIdForObject(this._getFragment()); - } - isTypeConditional(): boolean { return this._isTypeConditional; } diff --git a/src/query/__tests__/RelayFragmentReference-test.js b/src/query/__tests__/RelayFragmentReference-test.js index e627d895a70bd..89cb83c20c247 100644 --- a/src/query/__tests__/RelayFragmentReference-test.js +++ b/src/query/__tests__/RelayFragmentReference-test.js @@ -14,6 +14,8 @@ var RelayTestUtils = require('RelayTestUtils'); RelayTestUtils.unmockRelay(); +jest.mock('warning'); + var GraphQL = require('GraphQL'); var Relay = require('Relay'); var RelayFragmentReference = require('RelayFragmentReference'); @@ -23,7 +25,10 @@ describe('RelayFragmentReference', () => { var route; beforeEach(() => { + jest.resetModuleRegistry(); + route = new RelayMetaRoute(''); + jest.addMatchers(RelayTestUtils.matchers); }); it('creates fragments with default variables', () => { @@ -136,4 +141,29 @@ describe('RelayFragmentReference', () => { expect(reference.getVariables(route, variables)).toEqual(customVariables); expect(prepareVariables).toBeCalledWith({size: 'default'}, route); }); + + it('warns if a variable is undefined', () => { + var node = Relay.QL`fragment on Node{id}`; + var reference = new RelayFragmentReference( + () => node, + {}, + { + dynamic: new GraphQL.CallVariable('dynamic'), + static: undefined, + } + ); + var variables = {}; + expect(reference.getFragment(variables)).toBe(node); + expect(reference.getVariables(route, variables)).toEqual({}); + expect([ + 'RelayFragmentReference: Variable `%s` is undefined in fragment `%s`.', + 'static', + node.name + ]).toBeWarnedNTimes(1); + expect([ + 'RelayFragmentReference: Variable `%s` is undefined in fragment `%s`.', + 'dynamic', + node.name + ]).toBeWarnedNTimes(1); + }); }); diff --git a/src/query/__tests__/buildRQL-test.js b/src/query/__tests__/buildRQL-test.js index 7f18cac20e6b5..8cb2919b6c1c8 100644 --- a/src/query/__tests__/buildRQL-test.js +++ b/src/query/__tests__/buildRQL-test.js @@ -45,7 +45,7 @@ describe('buildRQL', () => { }); describe('Fragment()', () => { - it('throws if the node is not a fragment', () => { + it('returns undefined if the node is not a fragment', () => { var builder = () => Relay.QL` query { node(id:"123") { @@ -63,7 +63,7 @@ describe('buildRQL', () => { ${invalid}, } `; - expect(() => buildRQL.Fragment(builder, [])).toFailInvariant( + expect(() => buildRQL.Fragment(builder, {})).toFailInvariant( 'RelayQL: Invalid fragment composition, use ' + '`${Child.getFragment(\'name\')}`.' ); @@ -78,7 +78,7 @@ describe('buildRQL', () => { }, } `; - var node = buildRQL.Fragment(builder, ['sizeVariable']); + var node = buildRQL.Fragment(builder, {sizeVariable: null}); expect(GraphQL.isFragment(node)).toBe(true); // Confirm that `${variables.sizeVariable}` is a variable by applying @@ -105,20 +105,22 @@ describe('buildRQL', () => { }, } `; - var node1 = buildRQL.Fragment(builder, ['sizeVariable']); - var node2 = buildRQL.Fragment(builder, ['sizeVariable']); + var node1 = buildRQL.Fragment(builder, {sizeVariable: null}); + var node2 = buildRQL.Fragment(builder, {sizeVariable: null}); expect(node1 === node2).toBe(true); }); }); describe('Query()', () => { - it('throws if the node is not a query', () => { + it('returns undefined if the node is not a query', () => { var builder = (Component, variables) => Relay.QL` fragment on Node { id, } `; - expect(buildRQL.Query(builder, MockContainer, ['id'])).toBe(undefined); + expect( + buildRQL.Query(builder, MockContainer, 'foo', {}) + ).toBe(undefined); }); it('creates queries with components and variables', () => { @@ -130,7 +132,7 @@ describe('buildRQL', () => { } } `; - var node = buildRQL.Query(builder, MockContainer, ['id']); + var node = buildRQL.Query(builder, MockContainer, 'foo', {id: null}); expect(GraphQL.isQuery(node)).toBe(true); // Confirm that `${variables.id}` is a variable by applying variable @@ -155,8 +157,8 @@ describe('buildRQL', () => { } } `; - var node1 = buildRQL.Query(builder, MockContainer, ['id']); - var node2 = buildRQL.Query(builder, MockContainer, ['id']); + var node1 = buildRQL.Query(builder, MockContainer, 'foo', {id: null}); + var node2 = buildRQL.Query(builder, MockContainer, 'foo', {id: null}); expect(node1 === node2).toBe(true); }); @@ -174,8 +176,8 @@ describe('buildRQL', () => { } } `; - var node1 = buildRQL.Query(builder, MockContainer, ['id']); - var node2 = buildRQL.Query(builder, MockContainer2, ['id']); + var node1 = buildRQL.Query(builder, MockContainer, 'foo', {id: null}); + var node2 = buildRQL.Query(builder, MockContainer2, 'foo', {id: null}); expect(node1 === node2).toBe(false); }); @@ -188,8 +190,8 @@ describe('buildRQL', () => { var node = buildRQL.Query( builder, MockContainer, - ['id'], - MockContainer.getFragment('foo') + 'foo', + {id: null}, ); expect(GraphQL.isQuery(node)).toBe(true); @@ -220,8 +222,8 @@ describe('buildRQL', () => { buildRQL.Query( builder, MockContainer, - [], - MockContainer.getFragment('foo') + 'foo', + {} ); }).toFailInvariant( 'Relay.QL: Expected query `viewer` to be empty. For example, use ' + diff --git a/src/query/buildRQL.js b/src/query/buildRQL.js index 1d6f63c0d5aa2..abe3b8669a845 100644 --- a/src/query/buildRQL.js +++ b/src/query/buildRQL.js @@ -16,18 +16,17 @@ var GraphQL = require('GraphQL'); var Map = require('Map'); var RelayQL = require('RelayQL'); -var RelayFragmentReference = require('RelayFragmentReference'); import type {RelayConcreteNode} from 'RelayQL'; import type {Params} from 'RelayRoute'; import type {RelayContainer, Variables} from 'RelayTypes'; +var filterObject = require('filterObject'); var invariant = require('invariant'); +var mapObject = require('mapObject'); export type FragmentBuilder = (variables: Variables) => RelayConcreteNode; export type QueryBuilder = - (Component: RelayContainer, params: Params) => RelayConcreteNode; - -type VariableNames = Array; + (Component?: RelayContainer, params?: Params) => RelayConcreteNode; // Cache results of executing fragment query builders. var fragmentCache = new Map(); @@ -65,11 +64,11 @@ function isDeprecatedCallWithArgCountGreaterThan( var buildRQL = { Fragment( fragmentBuilder: FragmentBuilder, - variableNames: VariableNames + values: Variables ): ?GraphQL.QueryFragment { var node = fragmentCache.get(fragmentBuilder); if (!node) { - var variables = toVariables(variableNames); + var variables = toVariables(values); if (isDeprecatedCallWithArgCountGreaterThan(fragmentBuilder, 1)) { // TODO: Delete legacy support, (_, query, variables) => query`...`. node = (fragmentBuilder: any)(undefined, RelayQL, variables); @@ -84,8 +83,8 @@ var buildRQL = { Query( queryBuilder: QueryBuilder, Component: any, - variableNames: VariableNames, - fragmentReference: RelayFragmentReference + queryName: string, + values: Variables ): ?GraphQL.Query { var componentCache = queryCache.get(queryBuilder); var node; @@ -96,14 +95,14 @@ var buildRQL = { node = componentCache.get(Component); } if (!node) { - var variables = toVariables(variableNames); + var variables = toVariables(values); if (isDeprecatedCallWithArgCountGreaterThan(queryBuilder, 2)) { // TODO: Delete legacy support, (Component, variables, rql) => rql`...`. node = queryBuilder(Component, variables, RelayQL); } else if (isDeprecatedCallWithArgCountGreaterThan(queryBuilder, 0)) { node = queryBuilder(Component, variables); } else { - node = queryBuilder(Component, variables); + node = queryBuilder(); if (GraphQL.isQuery(node) && node.fragments.length === 0) { var rootCall = node.calls[0]; if (!node.fields.every(field => field.fields.length === 0)) { @@ -114,11 +113,14 @@ var buildRQL = { rootCall.name ); } + var fragmentValues = filterObject(values, (_, name) => + Component.hasVariable(name) + ); node = new GraphQL.Query( rootCall.name, rootCall.value, node.fields, - [fragmentReference], + [Component.getFragment(queryName, fragmentValues)], node.metadata, node.name ); @@ -133,14 +135,10 @@ var buildRQL = { }, }; -function toVariables(variableNames: VariableNames): { +function toVariables(variables: Variables): { [key: string]: GraphQL.CallVariable; } { - var variables = {}; - variableNames.forEach(name => { - variables[name] = new GraphQL.CallVariable(name); - }); - return variables; + return mapObject(variables, (_, name) => new GraphQL.CallVariable(name)); } module.exports = buildRQL;