Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate fragments with variables and reflect that in naming #2920

Merged
merged 1 commit into from Feb 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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