Skip to content

Remove deprecated 'allowedLegacyNames' property of 'GraphQLSchema' #2129

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

Merged
merged 1 commit into from
Aug 26, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 0 additions & 20 deletions src/type/__tests__/schema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,23 +269,13 @@ describe('Type System: Schema', () => {
).to.equal(undefined);
});

it('configures the schema for allowed legacy names', () => {
expect(
new GraphQLSchema({
allowedLegacyNames: ['__badName'],
}).__allowedLegacyNames,
).to.deep.equal(['__badName']);
});

it('checks the configuration for mistakes', () => {
// $DisableFlowOnNegativeTest
expect(() => new GraphQLSchema(() => null)).to.throw();
// $DisableFlowOnNegativeTest
expect(() => new GraphQLSchema({ types: {} })).to.throw();
// $DisableFlowOnNegativeTest
expect(() => new GraphQLSchema({ directives: {} })).to.throw();
// $DisableFlowOnNegativeTest
expect(() => new GraphQLSchema({ allowedLegacyNames: {} })).to.throw();
});
});

Expand Down Expand Up @@ -342,15 +332,6 @@ describe('Type System: Schema', () => {
).to.deep.equal([]);
});

it('still configures the schema for allowed legacy names', () => {
expect(
new GraphQLSchema({
assumeValid: true,
allowedLegacyNames: ['__badName'],
}).__allowedLegacyNames,
).to.deep.equal(['__badName']);
});

it('does not check the configuration for mistakes', () => {
const config = () => null;
config.assumeValid = true;
Expand All @@ -364,7 +345,6 @@ describe('Type System: Schema', () => {
assumeValid: true,
types: {},
directives: { reduce: () => [] },
allowedLegacyNames: {},
}),
).to.not.throw();
});
Expand Down
37 changes: 0 additions & 37 deletions src/type/__tests__/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,43 +463,6 @@ describe('Type System: Objects must have fields', () => {
},
]);
});

it('accepts an Object type with explicitly allowed legacy named fields', () => {
const schemaBad = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
});
const schemaOk = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
allowedLegacyNames: ['__badName'],
});
expect(validateSchema(schemaBad)).to.deep.equal([
{
message:
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
},
]);
expect(validateSchema(schemaOk)).to.deep.equal([]);
});

it('throws with bad value for explicitly allowed legacy names', () => {
expect(
() =>
new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: { __badName: { type: GraphQLString } },
}),
// $DisableFlowOnNegativeTest
allowedLegacyNames: true,
}),
).to.throw('"allowedLegacyNames" must be Array if provided but got: true.');
});
});

describe('Type System: Fields args must be properly named', () => {
Expand Down
19 changes: 0 additions & 19 deletions src/type/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ export class GraphQLSchema {
_possibleTypeMap: ObjMap<ObjMap<boolean>>;
// Used as a cache for validateSchema().
__validationErrors: ?$ReadOnlyArray<GraphQLError>;
// Referenced by validateSchema().
__allowedLegacyNames: $ReadOnlyArray<string>;

constructor(config: GraphQLSchemaConfig): void {
// If this schema was built from a source known to be valid, then it may be
Expand All @@ -156,18 +154,12 @@ export class GraphQLSchema {
'"directives" must be Array if provided but got: ' +
`${inspect(config.directives)}.`,
);
devAssert(
!config.allowedLegacyNames || Array.isArray(config.allowedLegacyNames),
'"allowedLegacyNames" must be Array if provided but got: ' +
`${inspect(config.allowedLegacyNames)}.`,
);
}

this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes;

this.__allowedLegacyNames = config.allowedLegacyNames || [];
this._queryType = config.query;
this._mutationType = config.mutation;
this._subscriptionType = config.subscription;
Expand Down Expand Up @@ -273,7 +265,6 @@ export class GraphQLSchema {
extensions: ?ReadOnlyObjMap<mixed>,
extensionASTNodes: $ReadOnlyArray<SchemaExtensionNode>,
assumeValid: boolean,
allowedLegacyNames: $ReadOnlyArray<string>,
|} {
return {
query: this.getQueryType(),
Expand All @@ -285,7 +276,6 @@ export class GraphQLSchema {
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes || [],
assumeValid: this.__validationErrors !== undefined,
allowedLegacyNames: this.__allowedLegacyNames,
};
}
}
Expand All @@ -304,15 +294,6 @@ export type GraphQLSchemaValidationOptions = {|
* Default: false
*/
assumeValid?: boolean,

/**
* If provided, the schema will consider fields or types with names included
* in this list valid, even if they do not adhere to the specification's
* schema validation rules.
*
* This option is provided to ease adoption and will be removed in v15.
*/
allowedLegacyNames?: ?$ReadOnlyArray<string>,
|};

export type GraphQLSchemaConfig = {|
Expand Down
5 changes: 0 additions & 5 deletions src/type/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,6 @@ function validateName(
context: SchemaValidationContext,
node: { +name: string, +astNode: ?ASTNode, ... },
): void {
// If a schema explicitly allows some legacy name which is no longer valid,
// allow it to be assumed valid.
if (context.schema.__allowedLegacyNames.indexOf(node.name) !== -1) {
return;
}
// Ensure names are valid, however introspection types opt out.
const error = isValidNameError(node.name, node.astNode || undefined);
if (error) {
Expand Down
11 changes: 0 additions & 11 deletions src/utilities/__tests__/buildASTSchema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -814,17 +814,6 @@ describe('Schema Builder', () => {
expect(errors).to.have.lengthOf.above(0);
});

it('Accepts legacy names', () => {
const sdl = `
type Query {
__badName: String
}
`;
const schema = buildSchema(sdl, { allowedLegacyNames: ['__badName'] });
const errors = validateSchema(schema);
expect(errors).to.have.lengthOf(0);
});

it('Rejects invalid SDL', () => {
const sdl = `
type Query {
Expand Down
18 changes: 0 additions & 18 deletions src/utilities/__tests__/buildClientSchema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,24 +480,6 @@ describe('Type System: build schema from introspection', () => {
expect(printSchema(clientSchema)).to.equal(sdl);
});

it('builds a schema with legacy names', () => {
const sdl = dedent`
type Query {
__badName: String
}
`;
const allowedLegacyNames = ['__badName'];
const schema = buildSchema(sdl, { allowedLegacyNames });

const introspection = introspectionFromSchema(schema);
const clientSchema = buildClientSchema(introspection, {
allowedLegacyNames,
});

expect(schema.__allowedLegacyNames).to.deep.equal(['__badName']);
expect(printSchema(clientSchema)).to.equal(sdl);
});

it('builds a schema aware of deprecation', () => {
const sdl = dedent`
enum Color {
Expand Down
42 changes: 0 additions & 42 deletions src/utilities/__tests__/extendSchema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1087,48 +1087,6 @@ describe('extendSchema', () => {
);
});

it('maintains configuration of the original schema object', () => {
const testSchemaWithLegacyNames = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: () => ({
id: { type: GraphQLID },
}),
}),
allowedLegacyNames: ['__badName'],
});
const ast = parse(`
extend type Query {
__badName: String
}
`);
const schema = extendSchema(testSchemaWithLegacyNames, ast);
expect(schema).to.deep.include({ __allowedLegacyNames: ['__badName'] });
});

it('adds to the configuration of the original schema object', () => {
const testSchemaWithLegacyNames = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
fields: () => ({
__badName: { type: GraphQLString },
}),
}),
allowedLegacyNames: ['__badName'],
});
const ast = parse(`
extend type Query {
__anotherBadName: String
}
`);
const schema = extendSchema(testSchemaWithLegacyNames, ast, {
allowedLegacyNames: ['__anotherBadName'],
});
expect(schema).to.deep.include({
__allowedLegacyNames: ['__badName', '__anotherBadName'],
});
});

describe('can add additional root operation types', () => {
it('does not automatically include common root type names', () => {
const schema = extendTestSchema(`
Expand Down
1 change: 0 additions & 1 deletion src/utilities/buildASTSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export function buildASTSchema(
directives,
astNode: schemaDef,
assumeValid: options && options.assumeValid,
allowedLegacyNames: options && options.allowedLegacyNames,
});

function getOperationTypes(schema: SchemaDefinitionNode) {
Expand Down
1 change: 0 additions & 1 deletion src/utilities/buildClientSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ export function buildClientSchema(
types: objectValues(typeMap),
directives,
assumeValid: options && options.assumeValid,
allowedLegacyNames: options && options.allowedLegacyNames,
});

// Given a type reference in introspection, return the GraphQLType instance.
Expand Down
6 changes: 0 additions & 6 deletions src/utilities/extendSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,6 @@ export function extendSchema(
}
}

// Support both original legacy names and extended legacy names.
const allowedLegacyNames = schemaConfig.allowedLegacyNames.concat(
(options && options.allowedLegacyNames) || [],
);

// Then produce and return a Schema with these types.
return new GraphQLSchema({
// Note: While this could make early assertions to get the correctly
Expand All @@ -208,7 +203,6 @@ export function extendSchema(
directives: getMergedDirectives(),
astNode: schemaDef || schemaConfig.astNode,
extensionASTNodes: schemaConfig.extensionASTNodes.concat(schemaExts),
allowedLegacyNames,
});

// Below are functions used for producing this schema that have closed over
Expand Down
10 changes: 0 additions & 10 deletions tstypes/type/schema.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export class GraphQLSchema {
extensions: Maybe<Readonly<Record<string, any>>>;
extensionASTNodes: ReadonlyArray<SchemaExtensionNode>;
assumeValid: boolean;
allowedLegacyNames: ReadonlyArray<string>;
};
}

Expand All @@ -85,15 +84,6 @@ export interface GraphQLSchemaValidationOptions {
* Default: false
*/
assumeValid?: boolean;

/**
* If provided, the schema will consider fields or types with names included
* in this list valid, even if they do not adhere to the specification's
* schema validation rules.
*
* This option is provided to ease adoption and will be removed in v15.
*/
allowedLegacyNames?: Maybe<ReadonlyArray<string>>;
}

export interface GraphQLSchemaConfig extends GraphQLSchemaValidationOptions {
Expand Down