Skip to content

Commit c4b2d5c

Browse files
alloyalloy
committed
[language-plugin] Don’t leave fragment name validation up to plugins.
1 parent 06bf77f commit c4b2d5c

File tree

6 files changed

+299
-233
lines changed

6 files changed

+299
-233
lines changed

‎packages/relay-compiler/core/RelayFindGraphQLTags.js‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* Copyright (c) 2013-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @providesModule RelayFindGraphQLTags
8+
* @flow
9+
* @format
10+
*/
11+
12+
'use strict';
13+
14+
const path = require('path');
15+
const util = require('util');
16+
const graphql = require('graphql');
17+
18+
const RelayCompilerCache = require('../util/RelayCompilerCache');
19+
const getModuleName = require('../util/getModuleName');
20+
21+
import type {File} from 'graphql-compiler';
22+
import type {GraphQLTag, GraphQLTagFinder} from '../language/RelayLanguagePluginInterface';
23+
24+
export type GraphQLTagFinderOptions = {|
25+
validateNames: boolean,
26+
|};
27+
28+
const cache = new RelayCompilerCache('RelayFindGraphQLTags', 'v1');
29+
30+
function memoizedFind(
31+
tagFinder: GraphQLTagFinder,
32+
text: string,
33+
baseDir: string,
34+
file: File,
35+
options: GraphQLTagFinderOptions,
36+
): Array<string> {
37+
return cache.getOrCompute(
38+
file.hash + (options.validateNames ? '1' : '0'),
39+
find.bind(null, tagFinder, text, path.join(baseDir, file.relPath), options),
40+
);
41+
}
42+
43+
function find(
44+
tagFinder: GraphQLTagFinder,
45+
text: string,
46+
absPath: string,
47+
{validateNames}: GraphQLTagFinderOptions,
48+
): Array<string> {
49+
const tags = tagFinder(text);
50+
if (validateNames) {
51+
const moduleName = getModuleName(absPath);
52+
tags.forEach(tag => validateTemplate(tag, moduleName, absPath));
53+
}
54+
return tags.map(tag => tag.template);
55+
}
56+
57+
function validateTemplate({template, keyName, sourceLocationOffset}: GraphQLTag, moduleName: string, filePath: string) {
58+
const ast = graphql.parse(new graphql.Source(template, filePath, sourceLocationOffset));
59+
ast.definitions.forEach((def: any) => {
60+
invariant(
61+
def.name,
62+
'RelayFindGraphQLTags: In module `%s`, a definition of kind `%s` requires a name.',
63+
moduleName,
64+
def.kind,
65+
);
66+
const definitionName = def.name.value;
67+
if (def.kind === 'OperationDefinition') {
68+
const operationNameParts = definitionName.match(
69+
/^(.*)(Mutation|Query|Subscription)$/,
70+
);
71+
invariant(
72+
operationNameParts && definitionName.startsWith(moduleName),
73+
'RelayFindGraphQLTags: Operation names in graphql tags must be prefixed ' +
74+
'with the module name and end in "Mutation", "Query", or ' +
75+
'"Subscription". Got `%s` in module `%s`.',
76+
definitionName,
77+
moduleName,
78+
);
79+
} else if (def.kind === 'FragmentDefinition') {
80+
if (keyName) {
81+
invariant(
82+
definitionName === moduleName + '_' + keyName,
83+
'RelayFindGraphQLTags: Container fragment names must be ' +
84+
'`<ModuleName>_<propName>`. Got `%s`, expected `%s`.',
85+
definitionName,
86+
moduleName + '_' + keyName,
87+
);
88+
} else {
89+
invariant(
90+
definitionName.startsWith(moduleName),
91+
'RelayFindGraphQLTags: Fragment names in graphql tags must be prefixed ' +
92+
'with the module name. Got `%s` in module `%s`.',
93+
definitionName,
94+
moduleName,
95+
);
96+
}
97+
}
98+
});
99+
}
100+
101+
// TODO: Not sure why this is defined here rather than imported, is it so that it doesn’t get stripped in prod?
102+
function invariant(condition, msg, ...args) {
103+
if (!condition) {
104+
throw new Error(util.format(msg, ...args));
105+
}
106+
}
107+
108+
module.exports = {
109+
find, // Exported for testing only.
110+
memoizedFind,
111+
};

‎packages/relay-compiler/core/RelaySourceModuleParser.js‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ const fs = require('fs');
1717
const invariant = require('invariant');
1818
const path = require('path');
1919

20-
const RelayCompilerCache = require('../util/RelayCompilerCache');
21-
2220
const {ASTCache, Profiler} = require('graphql-compiler');
2321

24-
import type {GraphQLTagFinder, GraphQLTagFinderOptions} from '../language/RelayLanguagePluginInterface';
22+
const {memoizedFind} = require('./RelayFindGraphQLTags')
23+
24+
import type {GraphQLTagFinder} from '../language/RelayLanguagePluginInterface';
2525
import type {File, FileFilter} from 'graphql-compiler';
2626
import type {DocumentNode} from 'graphql';
2727

@@ -32,22 +32,7 @@ const FIND_OPTIONS = {
3232
};
3333

3434
module.exports = (tagFinder: GraphQLTagFinder) => {
35-
const cache = new RelayCompilerCache('RelaySourceModuleParser.memoizedTagFinder', 'v1');
36-
37-
function memoizedTagFinder(
38-
text: string,
39-
baseDir: string,
40-
file: File,
41-
options: GraphQLTagFinderOptions,
42-
): Array<string> {
43-
return cache.getOrCompute(
44-
file.hash + (options.validateNames ? '1' : '0'),
45-
() => {
46-
const absPath = path.join(baseDir, file.relPath);
47-
return tagFinder(text, absPath, options);
48-
},
49-
);
50-
}
35+
const memoizedTagFinder = memoizedFind.bind(null, tagFinder);
5136

5237
// Throws an error if parsing the file fails
5338
function parseFile(baseDir: string, file: File): ?DocumentNode {

‎packages/relay-compiler/core/__tests__/RelayFindGraphQLTags-test.js‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/**
2+
* Copyright (c) 2013-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
* @emails oncall+relay
9+
*/
10+
11+
'use strict';
12+
13+
const RelayFindGraphQLTags = require('RelayFindGraphQLTags');
14+
const FindGraphQLTags = require('FindGraphQLTags');
15+
16+
import type {GraphQLTagFinderOptions} from 'RelayFindGraphQLTags';
17+
18+
describe('RelayFindGraphQLTags', () => {
19+
function find(text, options: GraphQLTagFinderOptions, absPath: string = '/path/to/FindGraphQLTags.js') {
20+
return RelayFindGraphQLTags.find(
21+
FindGraphQLTags.find,
22+
text,
23+
absPath,
24+
options,
25+
);
26+
}
27+
28+
describe('query validation', () => {
29+
it('prints correct file numbers in errors', () => {
30+
expect(() => {
31+
find(
32+
'const foo = 1;\n' +
33+
'foo(graphql`\n' +
34+
' fragment FindGraphQLTags on User {\n' +
35+
' ?\n' +
36+
' id\n' +
37+
' }\n' +
38+
'`);\n',
39+
{validateNames: true}
40+
);
41+
}).toThrow('Syntax Error: Cannot parse the unexpected character "?".');
42+
});
43+
});
44+
45+
describe('query name validation', () => {
46+
it('throws for invalid query names', () => {
47+
expect(() =>
48+
find(
49+
'graphql`query NotModuleName { me { id } }`;',
50+
{validateNames: true},
51+
),
52+
).toThrow(
53+
'FindGraphQLTags: Operation names in graphql tags must be prefixed with ' +
54+
'the module name and end in "Mutation", "Query", or "Subscription". ' +
55+
'Got `NotModuleName` in module `FindGraphQLTags`.',
56+
);
57+
});
58+
59+
it('does not validate names when options is not set', () => {
60+
find(
61+
'graphql`query NotModuleName { me { id } }`;',
62+
{validateNames: false},
63+
);
64+
});
65+
66+
it('parses queries with valid names', () => {
67+
expect(
68+
find(
69+
'graphql`query FindGraphQLTagsQuery { me { id } }`;',
70+
{validateNames: true},
71+
),
72+
).toEqual(['query FindGraphQLTagsQuery { me { id } }']);
73+
});
74+
75+
it('parses queries with valid names from filepath', () => {
76+
expect(
77+
find(
78+
'graphql`query TestComponentQuery { me { id } }`;',
79+
{validateNames: true},
80+
'./PathTo/SuperDuper/TestComponent.js',
81+
),
82+
).toEqual(['query TestComponentQuery { me { id } }']);
83+
expect(
84+
find(
85+
'graphql`query TestComponentQuery { me { id } }`;',
86+
{validateNames: true},
87+
'./PathTo/SuperDuper/TestComponent.react.js',
88+
),
89+
).toEqual(['query TestComponentQuery { me { id } }']);
90+
expect(
91+
find(
92+
'graphql`query TestComponentQuery { me { id } }`;',
93+
{validateNames: true},
94+
'./PathTo/SuperDuper/TestComponent.react.jsx',
95+
),
96+
).toEqual(['query TestComponentQuery { me { id } }']);
97+
expect(
98+
find(
99+
'graphql`query TestComponentQuery { me { id } }`;',
100+
{validateNames: true},
101+
'./PathTo/SuperDuper/TestComponent/index.js',
102+
),
103+
).toEqual(['query TestComponentQuery { me { id } }']);
104+
});
105+
106+
it('throws for invalid top-level fragment names', () => {
107+
expect(() =>
108+
find(
109+
'graphql`fragment NotModuleName on User { name }`;',
110+
{validateNames: true},
111+
),
112+
).toThrow(
113+
'FindGraphQLTags: Fragment names in graphql tags ' +
114+
'must be prefixed with the module name. Got ' +
115+
'`NotModuleName` in module `FindGraphQLTags`.',
116+
);
117+
});
118+
119+
it('parses top-level fragments with valid names', () => {
120+
expect(
121+
find(
122+
'graphql`fragment FindGraphQLTags on User { name }`;',
123+
{validateNames: true},
124+
),
125+
).toEqual(['fragment FindGraphQLTags on User { name }']);
126+
});
127+
128+
it('throws for invalid container fragment names', () => {
129+
expect(() =>
130+
find(`
131+
createFragmentContainer(Foo, {
132+
foo: graphql\`fragment FindGraphQLTags_notFoo on User { name }\`,
133+
});
134+
`, {validateNames: true}),
135+
).toThrow(
136+
'FindGraphQLTags: Container fragment names must be ' +
137+
'`<ModuleName>_<propName>`. Got `FindGraphQLTags_notFoo`, expected ' +
138+
'`FindGraphQLTags_foo`.',
139+
);
140+
});
141+
142+
it('parses container fragments with valid names', () => {
143+
expect(
144+
find(`
145+
createFragmentContainer(Foo, {
146+
foo: graphql\`fragment FindGraphQLTags_foo on User { name }\`,
147+
});
148+
`, {validateNames: true}),
149+
).toEqual(['fragment FindGraphQLTags_foo on User { name }']);
150+
});
151+
});
152+
});

‎packages/relay-compiler/language/RelayLanguagePluginInterface.js‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
'use strict';
1313

14-
import type {File, IRTransform, Root, Fragment} from 'graphql-compiler';
14+
import type {IRTransform, Root, Fragment} from 'graphql-compiler';
1515
import type {FormatModule} from '../codegen/writeRelayGeneratedFile';
1616

1717
export type TypeGenerator = {
@@ -20,15 +20,20 @@ export type TypeGenerator = {
2020
generate: (node: Root | Fragment, options: any) => string,
2121
};
2222

23-
export type GraphQLTagFinderOptions = {|
24-
validateNames: boolean,
25-
|};
23+
export type GraphQLTag = {
24+
keyName: ?string,
25+
template: string,
26+
sourceLocationOffset: {
27+
/* TODO: Is this also expected to yse 1-based index? */
28+
line: number,
29+
/* Should use 1-based index */
30+
column: number,
31+
}
32+
};
2633

2734
export type GraphQLTagFinder = (
2835
text: string,
29-
filePath: string,
30-
options: GraphQLTagFinderOptions,
31-
) => Array<string>;
36+
) => Array<GraphQLTag>;
3237

3338
export type PluginInterface = {
3439
inputExtensions: string[],

0 commit comments

Comments
 (0)