cjoudrey / graphql-schema-linter

Validate GraphQL schema definitions against a set of rules
MIT License
694 stars 62 forks source link

AST directly to validateSchema instead of a string #202

Open gagoar opened 4 years ago

gagoar commented 4 years ago

Hi! I've been writing a graphql-schema-linter extension for vscode and so far works with the latest changes in configuration, one nice feature I've been needing in order to speed up the extension and do more interesting things with it, is to be able to create my own AST (and merge the incoming changes instead of reparse the entire schema) is to use validateSchema and pass the AST instead of the this.getSchema() that is a concatenated string.

In order to do this, I would like to know if I should extend the signature of validateSchema to be able to take the AST as well as the string or should I create a different function to do this independiently ?

another possibility I can think of is to use the Configuration object and expand there behaviour.

Thanks.

cjoudrey commented 4 years ago

I've been writing a graphql-schema-linter extension for vscode

That sounds awesome! 🎉

In order to do this, I would like to know if I should extend the signature of validateSchema to be able to take the AST as well as the string or should I create a different function to do this independiently ?

At the moment, validateSchemaDefinition accepts schemaDefinition which is a string.

I wouldn't mind if you split that function into two. Something along the lines of:

export function validateSchemaDefinition(
  schemaDefinition,
  rules,
  configuration
) {
  let ast;

  let parseOptions = {};
  if (configuration.getOldImplementsSyntax()) {
    parseOptions.allowLegacySDLImplementsInterfaces = true;
  }

  try {
    ast = parse(schemaDefinition, parseOptions);
  } catch (e) {
    if (e instanceof GraphQLError) {
      e.ruleName = 'graphql-syntax-error';

      return [e];
    } else {
      throw e;
    }
  }

  return validateSchemaDefinitionAST(ast, rules, configuration)
}

export function validateSchemaDefinitionAST(
  ast,
  rules,
  configuration
) {
  // move the rest of the code from `validateSchemaDefinition` into here. 
}

Would that solve your issue?

gagoar commented 4 years ago

yea, that was pretty much what I was going for :)

I will work on this during the weekend.

my idea is to get closer to have something where I can play with the AST. Furthermore, part of my hopes with the extension is to have a language server that will be useful for every editor, not only for vscode.

I will soon share the PR.