sqlmancer icon indicating copy to clipboard operation
sqlmancer copied to clipboard

Replace directives with annotations

Open danielrearden opened this issue 4 years ago • 2 comments

The library currently makes use of schema directives for both transforming the schema and providing metadata about the underlying database. This works well enough, but will make it difficult to integrate Sqlmancer with code-first libraries (or for that matter, vanilla GraphQL.js) without publishing plugins specific to those libraries. Schema directives were never meant to be used for relaying metadata and using them this way has been demonstrably fragile, with AST information being "lost in translation" in various contexts.

Putting annotations inside descriptions (which could then be stripped when transforming the schema), would address these points. The schema might be a bit more verbose this way, but in some ways it would also be cleaner since all the info would just be moved above the type or field instead of getting shoved inline.

Compare:

type Query
  @sqlmancer(
    dialect: POSTGRES
    transformFieldNames: SNAKE_CASE
    customScalars: { JSON: ["JSON", "JSONObject"], Date: ["DateTime"] }
  ) {
  actors: Actor @paginate @many(model: "Actor")
}

with

"""
@sqlmancer {
  dialect: 'POSTGRES',
  transformFieldNames: 'SNAKE_CASE',
  customScalars: { JSON: ['JSON'], Date: ['DateTime'] },
}
"""
type Query {
  """
  @paginate
  @many { model: 'Actor' }
  """
  actors: Actor
}

The biggest hurdle with this approach is that there is no standard way of doing annotations like this. Whatever syntax is used needs to support multi-line strings (for example, for CTE definitions), so existing libraries like graphql-metadata and graphql-annotations are out.

danielrearden avatar May 21 '20 03:05 danielrearden

Initial pass available in this branch.

danielrearden avatar May 23 '20 21:05 danielrearden

I do not understand why passing dialect to the query object. Isn't that the role of each model? Esepcially if multiple clients are to be supported? And the goes with transformFieldNames. In short :

"""
@sqlmancer {
  customScalars: { JSON: ['JSON'], Date: ['DateTime'] },
}
"""
type Query {
  """
  @paginate
  @many { model: 'Actor' }
  """
  actors: Actor
}

The dialect is inferred by the model's Knex instance (see issue #15) and whether to convert the field names with a special case should be specified per model definition, not the actual query. Because if the type Query definition has many different models, connected to different tables and databases, then setting these properties at the Query level violates the contract of that definition; each model is in charge of their own dialects and field transformations.

yanickrochon avatar Jun 18 '20 15:06 yanickrochon