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

Request: consistent sorting within exported GQL schema #989

Closed
1 of 3 tasks
marshall007 opened this issue Feb 6, 2019 · 2 comments
Closed
1 of 3 tasks

Request: consistent sorting within exported GQL schema #989

marshall007 opened this issue Feb 6, 2019 · 2 comments

Comments

@marshall007
Copy link
Contributor

I'm submitting a ...

  • bug report
  • feature request
  • question

PostGraphile version: v4.3.2

Current behavior:

Since the auto-generated connections all take mostly the same parameters, git easily confuses the adding/removing of types in the schema as individual line changes. Taking this partial diff as an example, the artifactsToVehicles connection didn't actually change, it was the requirements connection that was added, but this is not immediately obvious:

diff --git a/cosmic.graphql b/cosmic.graphql
index ade3731..1ac8a04 100644
--- a/cosmic.graphql
+++ b/cosmic.graphql
@@ -197,6 +197,35 @@ type Artifact implements Node {
     condition: PartConfigurationCondition
   ): PartConfigurationsConnection!
 
+  """Reads and enables pagination through a set of `ArtifactsToVehicle`."""
+  artifactsToVehicles(
+    """Only read the first `n` values of the set."""
+    first: Int
+
+    """Only read the last `n` values of the set."""
+    last: Int
+
+    """
+    Skip the first `n` values from our `after` cursor, an alternative to cursor
+    based pagination. May not be used with `last`.
+    """
+    offset: Int
+
+    """Read all values in the set before (above) this cursor."""
+    before: Cursor
+
+    """Read all values in the set after (below) this cursor."""
+    after: Cursor
+
+    """The method to use when ordering `ArtifactsToVehicle`."""
+    orderBy: [ArtifactsToVehiclesOrderBy!] = [NATURAL]
+
+    """
+    A condition to be used in determining which values should be returned by the collection.
+    """
+    condition: ArtifactsToVehicleCondition
+  ): ArtifactsToVehiclesConnection!
+
   """Reads and enables pagination through a set of `ChangeSetApproval`."""
   changeSetApprovals(
     """Only read the first `n` values of the set."""
@@ -226,8 +255,8 @@ type Artifact implements Node {
     condition: ChangeSetApprovalCondition
   ): ChangeSetApprovalsConnection!
 
-  """Reads and enables pagination through a set of `ArtifactsToVehicle`."""
-  artifactsToVehicles(
+  """Reads and enables pagination through a set of `Requirement`."""
+  requirements(
     """Only read the first `n` values of the set."""
     first: Int
 
@@ -246,14 +275,35 @@ type Artifact implements Node {
     """Read all values in the set after (below) this cursor."""
     after: Cursor
 
-    """The method to use when ordering `ArtifactsToVehicle`."""
-    orderBy: [ArtifactsToVehiclesOrderBy!] = [NATURAL]
+    """The method to use when ordering `Requirement`."""
+    orderBy: [RequirementsOrderBy!] = [NATURAL]
 
     """
     A condition to be used in determining which values should be returned by the collection.
     """
-    condition: ArtifactsToVehicleCondition
-  ): ArtifactsToVehiclesConnection!
+    condition: RequirementCondition
+  ): RequirementsConnection!

Expected behavior:

When using exportGqlSchemaPath, the server should sort the generated output within each block so that the output remains mostly the same when types are added and removed from the schema.

@marshall007
Copy link
Contributor Author

Workaround available (borrowing from #512 and graphql/graphql-js#941). I think this should be implemented as default behavior (perhaps with a --export-schema-preserve-order to keep current behavior).

import { writeFileSync } from 'fs';
import { parse, buildASTSchema, GraphQLSchema } from 'graphql';
import { printSchema } from 'graphql/utilities';
import { resolve } from 'path';
import { createPostGraphileSchema } from 'postgraphile';

import {
  common,
  PG_POOL,
  PG_SCHEMA,
  EXPORT_GQL_SCHEMA_PATH,
} from './config';

createPostGraphileSchema(PG_POOL, PG_SCHEMA, common)
.then((original) => {
  const filepath = resolve(EXPORT_GQL_SCHEMA_PATH);
  console.log(`Writing GQL schema: ${filepath}`);
  writeFileSync(filepath, printSchemaOrdered(original));
  process.exit(0);
})
.catch(() => {
  console.error('Failed to generate GraphQL schema!');
  process.exit(1);
});

function printSchemaOrdered(original: GraphQLSchema) {
  // Clone schema so we don't damage anything
  const schema = buildASTSchema(parse(printSchema(original)));

  const typeMap = schema.getTypeMap();
  Object.keys(typeMap).forEach(name => {
    const Type = typeMap[name];

    // Object?
    if ('getFields' in Type) {
      const fields = Type.getFields();
      const keys = Object.keys(fields).sort();
      keys.forEach(key => {
        const value = fields[key];

        // Move the key to the end of the object
        delete fields[key];
        fields[key] = value;

        // Sort args
        if ('args' in value) {
          value.args.sort((a, b) => a.name.localeCompare(b.name));
        }
      });
    }

    // Enum?
    if ('getValues' in Type) {
      Type.getValues().sort((a, b) => a.name.localeCompare(b.name));
    }
  });

  return printSchema(schema);
};

@benjie
Copy link
Member

benjie commented Feb 6, 2019

I'm not sure this should be default behaviour, the ordering generated by the schema often makes more sense (e.g. nodeId is the first entry, then columns, then relations, computed fields, etc). I think it should be made easier, however, especially when doing git diff. You may find that using git's patience algorithm reduces the mess.

I'm not keen to add more command-line flags, I think this is better served by a small generic utility that sorts a GraphQL schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants