Skip to content

Commit

Permalink
Move NoUnusedVariablesRule to RelayTransforms
Browse files Browse the repository at this point in the history
Reviewed By: josephsavona

Differential Revision: D17796064

fbshipit-source-id: 40844ed7b8883616cc1214a9036ea950cd9dcb30
  • Loading branch information
alunyov authored and facebook-github-bot committed Oct 10, 2019
1 parent 6946e85 commit 3dd79e8
Show file tree
Hide file tree
Showing 28 changed files with 523 additions and 89 deletions.
15 changes: 6 additions & 9 deletions packages/relay-compiler/codegen/RelayFileWriter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const CodegenDirectory = require('./CodegenDirectory');
const CompilerContext = require('../core/GraphQLCompilerContext');
const Profiler = require('../core/GraphQLCompilerProfiler');
const RelayParser = require('../core/RelayParser');
const RelayValidator = require('../core/RelayValidator');

const compileRelayArtifacts = require('./compileRelayArtifacts');
const crypto = require('crypto');
Expand Down Expand Up @@ -103,14 +102,12 @@ function compileAll({
|}) {
// Verify using local and global rules, can run global verifications here
// because all files are processed together
let validationRules = RelayValidator.GLOBAL_RULES;
if (extraValidationRules) {
validationRules = [
...validationRules,
...(extraValidationRules.LOCAL_RULES || []),
...(extraValidationRules.GLOBAL_RULES || []),
];
}
const validationRules = extraValidationRules
? [
...(extraValidationRules.LOCAL_RULES || []),
...(extraValidationRules.GLOBAL_RULES || []),
]
: [];

const definitions = ASTConvert.convertASTDocumentsWithBase(
schema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ type Foo {

exports[`compileRelayArtifacts matches expected output: client-fields-on-roots.graphql 1`] = `
~~~~~~~~~~ INPUT ~~~~~~~~~~
query FooQuery($id: ID!, $arg: String) {
query FooQuery($id: ID!) {
client_root_field

node(id: $id) {
Expand Down Expand Up @@ -424,12 +424,6 @@ extend type Subscription {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "arg",
"type": "String",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -480,12 +474,6 @@ extend type Subscription {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "arg",
"type": "String",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -4299,7 +4287,7 @@ fragment FeedbackComments_feedback on Feedback {

exports[`compileRelayArtifacts matches expected output: connection-resolver-field-stream-if-default-true.graphql 1`] = `
~~~~~~~~~~ INPUT ~~~~~~~~~~
query QueryWithConnectionField($id: ID!, $enableStream: Boolean) {
query QueryWithConnectionField($id: ID!) {
feedback: node(id: $id) {
...FeedbackComments_feedback
}
Expand Down Expand Up @@ -4336,12 +4324,6 @@ fragment FeedbackComments_feedback on Feedback {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "enableStream",
"type": "Boolean",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -4378,12 +4360,6 @@ fragment FeedbackComments_feedback on Feedback {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "enableStream",
"type": "Boolean",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -4678,7 +4654,7 @@ fragment FeedbackComments_feedback on Feedback {

exports[`compileRelayArtifacts matches expected output: connection-resolver-field-stream-if-explicit-true.graphql 1`] = `
~~~~~~~~~~ INPUT ~~~~~~~~~~
query QueryWithConnectionField($id: ID!, $enableStream: Boolean) {
query QueryWithConnectionField($id: ID!) {
feedback: node(id: $id) {
...FeedbackComments_feedback
}
Expand Down Expand Up @@ -4716,12 +4692,6 @@ fragment FeedbackComments_feedback on Feedback {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "enableStream",
"type": "Boolean",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -4758,12 +4728,6 @@ fragment FeedbackComments_feedback on Feedback {
"name": "id",
"type": "ID!",
"defaultValue": null
},
{
"kind": "LocalArgument",
"name": "enableStream",
"type": "Boolean",
"defaultValue": null
}
],
"selections": [
Expand Down Expand Up @@ -19865,7 +19829,7 @@ query UnionQuery {
exports[`compileRelayArtifacts matches expected output: unknown-root-variable-in-fragment.invalid.graphql 1`] = `
~~~~~~~~~~ INPUT ~~~~~~~~~~
# expected-to-throw
query TestQuery($id: ID!, $pictureSize: [Int] = [128]) {
query TestQuery($id: ID!) {
node(id: $id) {
id
...Profile @relay(mask: false)
Expand Down Expand Up @@ -19894,7 +19858,7 @@ Error: Variable '$includeFriends' is not in scope.

Source: GraphQL request (2:1)
1: # expected-to-throw
2: query TestQuery($id: ID!, $pictureSize: [Int] = [128]) {
2: query TestQuery($id: ID!) {
^
3: node(id: $id) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
query FooQuery($id: ID!, $arg: String) {
query FooQuery($id: ID!) {
client_root_field

node(id: $id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
query QueryWithConnectionField($id: ID!, $enableStream: Boolean) {
query QueryWithConnectionField($id: ID!) {
feedback: node(id: $id) {
...FeedbackComments_feedback
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
query QueryWithConnectionField($id: ID!, $enableStream: Boolean) {
query QueryWithConnectionField($id: ID!) {
feedback: node(id: $id) {
...FeedbackComments_feedback
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# expected-to-throw
query TestQuery($id: ID!, $pictureSize: [Int] = [128]) {
query TestQuery($id: ID!) {
node(id: $id) {
id
...Profile @relay(mask: false)
Expand Down
3 changes: 3 additions & 0 deletions packages/relay-compiler/core/RelayIRTransforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const SkipRedundantNodesTransform = require('../transforms/SkipRedundantNodesTra
const SkipUnreachableNodeTransform = require('../transforms/SkipUnreachableNodeTransform');
const SkipUnusedVariablesTransform = require('../transforms/SkipUnusedVariablesTransform');
const ValidateGlobalVariablesTransform = require('../transforms/ValidateGlobalVariablesTransform');
const ValidateUnusedVariablesTransform = require('../transforms/ValidateUnusedVariablesTransform');

import type {IRTransform} from './GraphQLCompilerContext';

Expand All @@ -49,6 +50,7 @@ const relaySchemaExtensions: $ReadOnlyArray<string> = [
RelayTestOperationTransform.SCHEMA_EXTENSION,
InlineDataFragmentTransform.SCHEMA_EXTENSION,
RelayFlowGenerator.SCHEMA_EXTENSION,
ValidateUnusedVariablesTransform.SCHEMA_EXTENSION,
];

// Transforms applied to both operations and fragments for both reading and
Expand Down Expand Up @@ -76,6 +78,7 @@ const relayFragmentTransforms: $ReadOnlyArray<IRTransform> = [
// Transforms applied to queries/mutations/subscriptions that are used for
// fetching data from the server and parsing those responses.
const relayQueryTransforms: $ReadOnlyArray<IRTransform> = [
ValidateUnusedVariablesTransform.transform,
RelayApplyFragmentArgumentTransform.transform,
ValidateGlobalVariablesTransform.transform,
RelayGenerateIDFieldTransform.transform,
Expand Down
19 changes: 1 addition & 18 deletions packages/relay-compiler/core/RelayValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const Profiler = require('./GraphQLCompilerProfiler');

const util = require('util');

const {NoUnusedVariablesRule, formatError} = require('graphql');
const {formatError} = require('graphql');

import type {Schema} from './Schema';
import type {DocumentNode, ValidationRule} from 'graphql';
Expand All @@ -41,23 +41,6 @@ function validateOrThrow(
}

module.exports = {
GLOBAL_RULES: [
/* Some rules are not enabled (potentially non-exhaustive)
*
* - KnownFragmentNamesRule: RelayClassic generates fragments at runtime,
* so RelayCompat queries might reference fragments unknown in build time.
* - NoFragmentCyclesRule: Because of @argumentDefinitions, this validation
* incorrectly flags a subset of fragments using @include/@skip as
* recursive.
* - NoUndefinedVariablesRule: Because of @argumentDefinitions, this
* validation incorrectly marks some fragment variables as undefined.
* - NoUnusedFragmentsRule: Queries generated dynamically with RelayCompat
* might use unused fragments.
* - OverlappingFieldsCanBeMergedRule: RelayClassic auto-resolves
* overlapping fields by generating aliases.
*/
NoUnusedVariablesRule,
],
validate: (Profiler.instrument(validateOrThrow, 'RelayValidator.validate'): (
schema: Schema,
document: DocumentNode,
Expand Down
6 changes: 1 addition & 5 deletions packages/relay-compiler/core/inferRootArgumentDefinitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,10 @@ function visit(
// Merge any root variables referenced by the spread fragment
// into this (parent) fragment's arguments.
for (const argDef of referencedFragmentArguments.values()) {
if (
argDef.kind === 'RootArgumentDefinition' &&
!argumentDefinitions.has(argDef.name)
) {
if (argDef.kind === 'RootArgumentDefinition') {
argumentDefinitions.set(argDef.name, argDef);
}
}
return false;
},
Argument(argument: Argument) {
if (argument.value.kind !== 'Variable') {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
*/

'use strict';

const inferRootArgumentDefinitions = require('../core/inferRootArgumentDefinitions');

const {
createCombinedError,
createUserError,
eachWithErrors,
} = require('../core/RelayCompilerError');

import type GraphQLCompilerContext from '../core/GraphQLCompilerContext';
import type {ArgumentDefinition} from '../core/GraphQLIR';

const SCHEMA_EXTENSION =
'directive @DEPRECATED__relay_ignore_unused_variables_error on QUERY | MUTATION | SUBSCRIPTION';

/**
* Validates that there are no unused variables in the operation.
* former `graphql-js`` NoUnusedVariablesRule
*/
function validateUnusedVariablesTransform(
context: GraphQLCompilerContext,
): GraphQLCompilerContext {
const contextWithUsedArguments = inferRootArgumentDefinitions(context);
const errors = eachWithErrors(context.documents(), node => {
if (node.kind !== 'Root') {
return;
}
const rootArgumentLocations = new Map(
node.argumentDefinitions.map(arg => [arg.name, arg.loc]),
);
const nodeWithUsedArguments = contextWithUsedArguments.getRoot(node.name);
const usedArguments = argumentDefinitionsToMap(
nodeWithUsedArguments.argumentDefinitions,
);
for (const usedArgumentName of usedArguments.keys()) {
rootArgumentLocations.delete(usedArgumentName);
}

const ignoreErrorDirective = node.directives.find(
({name}) => name === 'DEPRECATED__relay_ignore_unused_variables_error',
);
if (rootArgumentLocations.size > 0 && !ignoreErrorDirective) {
const isPlural = rootArgumentLocations.size > 1;
throw createUserError(
`Variable${isPlural ? 's' : ''} '$${Array.from(
rootArgumentLocations.keys(),
).join("', '$")}' ${isPlural ? 'are' : 'is'} never used in operation '${
node.name
}'.`,
Array.from(rootArgumentLocations.values()),
);
}
if (rootArgumentLocations.size === 0 && ignoreErrorDirective) {
throw createUserError(
"Invalid usage of '@DEPRECATED__relay_ignore_unused_variables_error.'" +
`No unused variables found in the query '${node.name}'`,
[ignoreErrorDirective.loc],
);
}
});
if (errors != null && errors.length !== 0) {
throw createCombinedError(errors);
}
return context;
}

function argumentDefinitionsToMap<T: ArgumentDefinition>(
argDefs: $ReadOnlyArray<T>,
): Map<string, T> {
const map = new Map();
for (const argDef of argDefs) {
map.set(argDef.name, argDef);
}
return map;
}

module.exports = {
transform: validateUnusedVariablesTransform,
SCHEMA_EXTENSION,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
* @emails oncall+relay
*/

'use strict';

const GraphQLCompilerContext = require('../../core/GraphQLCompilerContext');
const Schema = require('../../core/Schema');
const ValidateUnusedVariablesTransform = require('../ValidateUnusedVariablesTransform');

const {transformASTSchema} = require('../../core/ASTConvert');
const {
TestSchema,
generateTestsFromFixtures,
parseGraphQLText,
} = require('relay-test-utils-internal');

generateTestsFromFixtures(
`${__dirname}/fixtures/ValidateUnusedVariablesTransform`,
text => {
const extendedSchema = transformASTSchema(TestSchema, [
ValidateUnusedVariablesTransform.SCHEMA_EXTENSION,
]);
const {definitions} = parseGraphQLText(extendedSchema, text);
const compilerSchema = Schema.DEPRECATED__create(
TestSchema,
extendedSchema,
);
return new GraphQLCompilerContext(compilerSchema)
.addAll(definitions)
.applyTransforms([ValidateUnusedVariablesTransform.transform])
.documents()
.map(doc => `${doc.name}: NO ERRORS.`)
.join('\n');
},
);
Loading

0 comments on commit 3dd79e8

Please sign in to comment.