Skip to content

Commit

Permalink
Deprecate fragments with variables and reflect that in naming (graphq…
Browse files Browse the repository at this point in the history
  • Loading branch information
IvanGoncharov committed Feb 20, 2021
1 parent 3638c2d commit 6054799
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/language/__tests__/parser-test.js
Expand Up @@ -363,11 +363,11 @@ describe('Parser', () => {
expect(result.loc).to.equal(undefined);
});

it('Experimental: allows parsing fragment defined variables', () => {
it('Legacy: allows parsing fragment defined variables', () => {
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';

expect(() =>
parse(document, { experimentalFragmentVariables: true }),
parse(document, { allowLegacyFragmentVariables: true }),
).to.not.throw();
expect(() => parse(document)).to.throw('Syntax Error');
});
Expand Down
10 changes: 4 additions & 6 deletions src/language/__tests__/printer-test.js
Expand Up @@ -117,12 +117,10 @@ describe('Printer: Query document', () => {
);
});

it('Experimental: prints fragment with variable directives', () => {
it('Legacy: prints fragment with variable directives', () => {
const queryASTWithVariableDirective = parse(
'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
{
experimentalFragmentVariables: true,
},
{ allowLegacyFragmentVariables: true },
);
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
fragment Foo($foo: TestType @test) on TestType @testDirective {
Expand All @@ -131,14 +129,14 @@ describe('Printer: Query document', () => {
`);
});

it('Experimental: correctly prints fragment defined variables', () => {
it('Legacy: correctly prints fragment defined variables', () => {
const fragmentWithVariable = parse(
`
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
id
}
`,
{ experimentalFragmentVariables: true },
{ allowLegacyFragmentVariables: true },
);
expect(print(fragmentWithVariable)).to.equal(dedent`
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
Expand Down
4 changes: 2 additions & 2 deletions src/language/__tests__/visitor-test.js
Expand Up @@ -438,10 +438,10 @@ describe('Visitor', () => {
]);
});

it('Experimental: visits variables defined in fragments', () => {
it('Legacy: visits variables defined in fragments', () => {
const ast = parse('fragment a($v: Boolean = false) on t { f }', {
noLocation: true,
experimentalFragmentVariables: true,
allowLegacyFragmentVariables: true,
});
const visited = [];

Expand Down
3 changes: 1 addition & 2 deletions src/language/ast.d.ts
Expand Up @@ -297,8 +297,7 @@ export interface FragmentDefinitionNode {
readonly kind: 'FragmentDefinition';
readonly loc?: Location;
readonly name: NameNode;
// Note: fragment variable definitions are experimental and may be changed
// or removed in the future.
// Note: fragment variable definitions are deprecated and will removed in v17.0.0
readonly variableDefinitions?: ReadonlyArray<VariableDefinitionNode>;
readonly typeCondition: NamedTypeNode;
readonly directives?: ReadonlyArray<DirectiveNode>;
Expand Down
3 changes: 1 addition & 2 deletions src/language/ast.js
Expand Up @@ -323,8 +323,7 @@ export type FragmentDefinitionNode = {|
+kind: 'FragmentDefinition',
+loc?: Location,
+name: NameNode,
// Note: fragment variable definitions are experimental and may be changed
// or removed in the future.
// Note: fragment variable definitions are deprecated and will removed in v17.0.0
+variableDefinitions?: $ReadOnlyArray<VariableDefinitionNode>,
+typeCondition: NamedTypeNode,
+directives?: $ReadOnlyArray<DirectiveNode>,
Expand Down
6 changes: 2 additions & 4 deletions src/language/parser.d.ts
Expand Up @@ -13,7 +13,7 @@ export interface ParseOptions {
noLocation?: boolean;

/**
* EXPERIMENTAL:
* @deprecated will be removed in the v17.0.0
*
* If enabled, the parser will understand and parse variable definitions
* contained in a fragment definition. They'll be represented in the
Expand All @@ -25,10 +25,8 @@ export interface ParseOptions {
* ...
* }
*
* Note: this feature is experimental and may change or be removed in the
* future.
*/
experimentalFragmentVariables?: boolean;
allowLegacyFragmentVariables?: boolean;
}

/**
Expand Down
10 changes: 4 additions & 6 deletions src/language/parser.js
Expand Up @@ -67,7 +67,7 @@ export type ParseOptions = {|
noLocation?: boolean,

/**
* EXPERIMENTAL:
* @deprecated will be removed in the v17.0.0
*
* If enabled, the parser will understand and parse variable definitions
* contained in a fragment definition. They'll be represented in the
Expand All @@ -79,10 +79,8 @@ export type ParseOptions = {|
* ...
* }
*
* Note: this feature is experimental and may change or be removed in the
* future.
*/
experimentalFragmentVariables?: boolean,
allowLegacyFragmentVariables?: boolean,
|};

/**
Expand Down Expand Up @@ -458,10 +456,10 @@ export class Parser {
parseFragmentDefinition(): FragmentDefinitionNode {
const start = this._lexer.token;
this.expectKeyword('fragment');
// Experimental support for defining variables within fragments changes
// Legacy support for defining variables within fragments changes
// the grammar of FragmentDefinition:
// - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
if (this._options?.experimentalFragmentVariables === true) {
if (this._options?.allowLegacyFragmentVariables === true) {
return {
kind: Kind.FRAGMENT_DEFINITION,
name: this.parseFragmentName(),
Expand Down
3 changes: 1 addition & 2 deletions src/language/visitor.d.ts
Expand Up @@ -78,8 +78,7 @@ export const QueryDocumentKeys: {
// prettier-ignore
FragmentDefinition: [
'name',
// Note: fragment variable definitions are experimental and may be changed
// or removed in the future.
// Note: fragment variable definitions are deprecated and will removed in v17.0.0
'variableDefinitions',
'typeCondition',
'directives',
Expand Down
3 changes: 1 addition & 2 deletions src/language/visitor.js
Expand Up @@ -67,8 +67,7 @@ export const QueryDocumentKeys: VisitorKeyMap<ASTKindToNode> = {
InlineFragment: ['typeCondition', 'directives', 'selectionSet'],
FragmentDefinition: [
'name',
// Note: fragment variable definitions are experimental and may be changed
// or removed in the future.
// Note: fragment variable definitions are deprecated and will removed in v17.0.0
'variableDefinitions',
'typeCondition',
'directives',
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/buildASTSchema.js
Expand Up @@ -98,7 +98,7 @@ export function buildSchema(
): GraphQLSchema {
const document = parse(source, {
noLocation: options?.noLocation,
experimentalFragmentVariables: options?.experimentalFragmentVariables,
allowLegacyFragmentVariables: options?.allowLegacyFragmentVariables,
});

return buildASTSchema(document, {
Expand Down

0 comments on commit 6054799

Please sign in to comment.