Format service SDL and keep multiline comment spacing (#7664)

.changeset/cool-ties-tell.md

@@ -0,0 +1,5 @@
+---
+'@graphql-hive/cli': patch
+---
+
+Do not minify multiline descriptions in SDL on check/push

.changeset/curly-dodos-retire.md

@@ -0,0 +1,5 @@
+---
+'hive': patch
+---
+
+Fix service SDL being printed on a single line in check and publish views

.prettierignore

@@ -18,6 +18,7 @@ __snapshots__/
 
 # test fixtures
 integration-tests/fixtures/init-invalid-schema.graphql
+integration-tests/fixtures/whitespace-oddity.graphql
 /target
 tmp
 

integration-tests/fixtures/whitespace-oddity.graphql

@@ -0,0 +1,13 @@
+"""
+Multi line comment:
+1. Foo
+2. Bar
+3. Should stay in a list format
+"""
+type Query { status: Status }
+
+# Another comment here
+enum Status {
+  ACTIVE INACTIVE
+  PENDING
+}

integration-tests/testkit/flow.ts

@@ -1207,6 +1207,8 @@ export function fetchLatestSchema(token: string) {
               deletedService
             }
           }
+          sdl
+          supergraph
           schemas {
             nodes {
               ... on SingleSchema {

integration-tests/tests/api/artifacts-cdn.spec.ts

@@ -326,7 +326,7 @@ function runArtifactsCDNTests(
         `artifact/${target.id}/services`,
       );
       expect(artifactContents.body).toMatchInlineSnapshot(
-        '[{"name":"ping","sdl":"type Query { ping: String }","url":"http://ping.com"}]',
+        `[{"name":"ping","sdl":"type Query {\\n  ping: String\\n}","url":"http://ping.com"}]`,
       );
 
       const cdnAccessResult = await createCdnAccess();
@@ -343,7 +343,7 @@ function runArtifactsCDNTests(
       expect(response.status).toBe(200);
       const body = await response.text();
       expect(body).toMatchInlineSnapshot(
-        '[{"name":"ping","sdl":"type Query { ping: String }","url":"http://ping.com"}]',
+        `[{"name":"ping","sdl":"type Query {\\n  ping: String\\n}","url":"http://ping.com"}]`,
       );
     });
 
@@ -375,7 +375,7 @@ function runArtifactsCDNTests(
         `artifact/${target.id}/services`,
       );
       expect(artifactContents.body).toMatchInlineSnapshot(
-        '[{"name":"ping","sdl":"type Query { ping: String }","url":"http://ping.com"}]',
+        `[{"name":"ping","sdl":"type Query {\\n  ping: String\\n}","url":"http://ping.com"}]`,
       );
 
       const cdnAccessResult = await createCdnAccess();
@@ -720,7 +720,7 @@ function runArtifactsCDNTests(
       expect(versionedResponse.headers.get('content-type')).toContain('application/json');
       const servicesBody = await versionedResponse.text();
       expect(servicesBody).toMatchInlineSnapshot(
-        '[{"name":"ping","sdl":"type Query { ping: String }","url":"http://ping.com"}]',
+        `[{"name":"ping","sdl":"type Query {\\n  ping: String\\n}","url":"http://ping.com"}]`,
       );
 
       // Verify the versioned S3 key exists

integration-tests/tests/api/schema/check.spec.ts

@@ -606,14 +606,14 @@ test.concurrent('failed check due to policy error is persisted', async ({ expect
             {
               node: {
                 end: {
-                  column: 17,
-                  line: 2,
+                  column: 11,
+                  line: 1,
                 },
                 message: 'Description is required for type "Query"',
                 ruleId: 'require-description',
                 start: {
-                  column: 12,
-                  line: 2,
+                  column: 6,
+                  line: 1,
                 },
               },
             },

integration-tests/tests/api/schema/publish.spec.ts

@@ -116,7 +116,10 @@ test.concurrent(
     expect(firstNode).toEqual(
       expect.objectContaining({
         commit: '2',
-        source: expect.stringContaining('type Query { ping: String @auth pong: String }'),
+        source: `type Query {
+  ping: String @auth
+  pong: String
+}`,
       }),
     );
     expect(firstNode).not.toEqual(
@@ -158,9 +161,14 @@ test.concurrent('directives should not be removed (federation)', async () => {
   expect(latestResult.latestVersion?.schemas.nodes[0]).toEqual(
     expect.objectContaining({
       commit: 'abc123',
-      source: expect.stringContaining(
-        `type Query { me: User } type User @key(fields: "id") { id: ID! name: String }`,
-      ),
+      source: `type Query {
+  me: User
+}
+
+type User @key(fields: "id") {
+  id: ID!
+  name: String
+}`,
     }),
   );
 });
@@ -194,9 +202,14 @@ test.concurrent('directives should not be removed (stitching)', async () => {
   expect(latestResult.latestVersion?.schemas.nodes[0]).toEqual(
     expect.objectContaining({
       commit: 'abc123',
-      source: expect.stringContaining(
-        `type Query { me: User } type User @key(selectionSet: "{ id }") { id: ID! name: String }`,
-      ),
+      source: `type Query {
+  me: User
+}
+
+type User @key(selectionSet: "{ id }") {
+  id: ID!
+  name: String
+}`,
     }),
   );
 });
@@ -229,9 +242,16 @@ test.concurrent('directives should not be removed (single)', async () => {
   expect(latestResult.latestVersion?.schemas.nodes[0]).toEqual(
     expect.objectContaining({
       commit: 'abc123',
-      source: expect.stringContaining(
-        `directive @auth on FIELD_DEFINITION type Query { me: User @auth } type User { id: ID! name: String }`,
-      ),
+      source: `directive @auth on FIELD_DEFINITION
+
+type Query {
+  me: User @auth
+}
+
+type User {
+  id: ID!
+  name: String
+}`,
     }),
   );
 });

integration-tests/tests/cli/schema.spec.ts

@@ -1102,3 +1102,74 @@ test.concurrent(
     }
   },
 );
+
+test.concurrent('schema:publish ignores SDL formatting', async ({ expect }) => {
+  const { createOrg } = await initSeed().createOwner();
+  const { inviteAndJoinMember, createProject, organization } = await createOrg();
+  await inviteAndJoinMember();
+  const { createTargetAccessToken, project, target } = await createProject(ProjectType.Federation);
+  const { secret, latestSchema } = await createTargetAccessToken({});
+
+  const targetSlug = [organization.slug, project.slug, target.slug].join('/');
+
+  await expect(
+    schemaPublish([
+      '--registry.accessToken',
+      secret,
+      '--author',
+      'Kamil',
+      '--target',
+      targetSlug,
+      '--service',
+      'whitespace',
+      '--url',
+      'https://example.graphql-hive.com/graphql',
+      'fixtures/whitespace-oddity.graphql',
+    ]),
+  ).resolves.toMatchInlineSnapshot(`
+      :::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::
+
+      stdout--------------------------------------------:
+      ✔ Published initial schema.
+      ℹ Available at http://__URL__
+    `);
+
+  const latest = await latestSchema();
+  expect(latest.latestVersion?.schemas.nodes?.[0]?.source).toMatchInlineSnapshot(`
+    """
+    Multi line comment:
+    1. Foo
+    2. Bar
+    3. Should stay in a list format
+    """
+    type Query {
+      status: Status
+    }
+
+    enum Status {
+      ACTIVE
+      INACTIVE
+      PENDING
+    }
+  `);
+
+  // API Schema maintains formatting
+  expect(latest.latestVersion?.sdl).toEqual(
+    expect.stringContaining(`"""
+Multi line comment:
+1. Foo
+2. Bar
+3. Should stay in a list format
+"""`),
+  );
+
+  // Supergraph maintains formatting
+  expect(latest.latestVersion?.supergraph).toEqual(
+    expect.stringContaining(`"""
+Multi line comment:
+1. Foo
+2. Bar
+3. Should stay in a list format
+"""`),
+  );
+});

packages/libraries/cli/src/helpers/schema.ts

@@ -167,6 +167,18 @@ export async function loadSchema(
   return print(concatAST(sources.map(s => s.document!)));
 }
 
-export function minifySchema(schema: string): string {
-  return schema.replace(/\s+/g, ' ').trim();
+export function minifySchema(schema: string) {
+  // Regex breakdown:
+  // 1. ("""[\s\S]*?""") -> Group 1: Triple-quoted blocks
+  // 2. #[^\r\n]* -> Matches comments starting with #
+  // 3. \s+              -> Matches one or more whitespaces
+  return schema
+    .replace(/(?:("""[\s\S]*?""")|#[^\r\n]*|\s+)/g, (match, group1) => {
+      // If it's a triple-quote block, return it exactly as is
+      if (group1) return group1;
+
+      // If it was a comment or whitespace(s), replace with a single space
+      return ' ';
+    })
+    .trim();
 }

packages/services/api/src/index.ts

@@ -18,7 +18,6 @@ export type {
   OrganizationBilling,
   OrganizationInvitation,
 } from './shared/entities';
-export { minifySchema } from './shared/schema';
 export { HiveError } from './shared/errors';
 export { ProjectType } from './__generated__/types';
 export type { AuthProviderType } from './__generated__/types';

packages/services/api/src/modules/schema/providers/schema-publisher.ts

@@ -614,9 +614,7 @@ export class SchemaPublisher {
               existing: latestVersion
                 ? toSingleSchemaInput(ensureSingleSchema(latestVersion.schemas))
                 : null,
-              incoming: {
-                sdl,
-              },
+              incoming: { sdl },
             });
             if ('result' in diffSchema) {
               proposalChanges = diffSchema.result ?? null;
@@ -627,9 +625,7 @@ export class SchemaPublisher {
         }
 
         checkResult = await this.models[ProjectType.SINGLE].check({
-          input: {
-            sdl: input.sdl,
-          },
+          input: { sdl },
           selector,
           latest: latestVersion
             ? {
@@ -1291,6 +1287,7 @@ export class SchemaPublisher {
             executor: () =>
               this.internalPublish({
                 ...input,
+                sdl: tryPrettifySDL(input.sdl),
                 checksum,
                 selector,
               }),

packages/services/api/src/shared/schema.ts

@@ -80,10 +80,6 @@ export const parseGraphQLSource = traceInlineSync(
   },
 );
 
-export function minifySchema(schema: string): string {
-  return schema.replace(/\s+/g, ' ').trim();
-}
-
 export function createConnection<TInput>(): {
   nodes(nodes: readonly TInput[]): readonly TInput[];
   total(nodes: readonly TInput[]): number;

scripts/seed-schemas.ts

@@ -87,6 +87,7 @@ const publishMutationDocument =
   `;
 
 async function publishSchema(args: { sdl: string; service?: string; target?: string }) {
+  const commit = `${Date.now()}`;
   const response = await fetch(graphqlEndpoint, {
     method: 'POST',
     headers: {
@@ -98,7 +99,7 @@ async function publishSchema(args: { sdl: string; service?: string; target?: str
       variables: {
         input: {
           author: 'MoneyBoy',
-          commit: '1977',
+          commit,
           sdl: args.sdl,
           service: args.service,
           url: `https://${args.service ? `${args.service}.` : ''}localhost/graphql`,
@@ -110,6 +111,7 @@ async function publishSchema(args: { sdl: string; service?: string; target?: str
   return response as {
     data: {
       schemaPublish: {
+        linkToWebsite: string;
         valid: boolean;
       } | null;
     } | null;
@@ -117,11 +119,15 @@ async function publishSchema(args: { sdl: string; service?: string; target?: str
   };
 }
 
+function minifySchema(schema: string): string {
+  return schema.replace(/\s+/g, ' ').trim();
+}
+
 async function single() {
   const schema = await loadSchema('scripts/seed-schemas/mono.graphql', {
     loaders: [new GraphQLFileLoader()],
   });
-  const sdl = printSchema(schema);
+  const sdl = minifySchema(printSchema(schema));
   const result = await publishSchema({
     sdl,
     target,
@@ -129,7 +135,7 @@ async function single() {
   if (result?.errors || result?.data?.schemaPublish?.valid !== true) {
     console.error(`Published schema is invalid.`);
   } else {
-    console.log(`Published successfully.`);
+    console.log(`Published successfully (${result.data.schemaPublish?.linkToWebsite})`);
   }
   return result;
 }
@@ -147,7 +153,7 @@ async function federation() {
       const service = d.location ? parsePath(d.location).name.replaceAll('.', '-') : undefined;
 
       const result = await publishSchema({
-        sdl: d.rawSDL,
+        sdl: minifySchema(d.rawSDL),
         service,
         target,
       });