fix(directives): allow multiline and leading spaces (#1669)

* fix: allow @directive definition multiline and leading spaces * test: better multiline and leading whitespaces tests * Fix assert directive --------- Co-authored-by: Michał Lytek <[email protected]>
MichalLytek · Apr 24, 2024 · 5690981 · 5690981
1 parent 3ba84c1
commit 5690981
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 ## Fixes
 
 - properly override fields of `@ArgsType` classes in deeply nested inheritance chain (#1644)
+- allow for leading spaces and multiline directives definitions in `@Directive` decorator (#1423)
 
 ## v2.0.0-beta.6
 

diff --git a/src/schema/definition-node.ts b/src/schema/definition-node.ts
@@ -18,7 +18,9 @@ import { type DirectiveMetadata } from "@/metadata/definitions";
 import { type SetRequired } from "@/typings";
 
 export function getDirectiveNode(directive: DirectiveMetadata): ConstDirectiveNode {
-  const { nameOrDefinition, args } = directive;
+  // Inline and trim start
+  const nameOrDefinition = directive.nameOrDefinition.replaceAll("\n", " ").trimStart();
+  const { args } = directive;
 
   if (nameOrDefinition === "") {
     throw new InvalidDirectiveError(

diff --git a/tests/functional/directives.ts b/tests/functional/directives.ts
@@ -582,6 +582,101 @@ describe("Directives", () => {
     });
   });
 
+  describe("multiline and leading white spaces", () => {
+    let schema: GraphQLSchema;
+    beforeAll(async () => {
+      @Resolver()
+      class SampleResolver {
+        @Directive("\n@test")
+        @Query()
+        multiline(): boolean {
+          return true;
+        }
+
+        @Directive(" @test")
+        @Query()
+        leadingWhiteSpaces(): boolean {
+          return true;
+        }
+
+        @Directive("\n @test")
+        @Query()
+        multilineAndLeadingWhiteSpaces(): boolean {
+          return true;
+        }
+
+        @Directive(`
+          @test(
+            argNonNullDefault: "argNonNullDefault",
+            argNullDefault: "argNullDefault",
+            argNull: "argNull"
+          )
+        `)
+        @Query()
+        rawMultilineAndLeadingWhiteSpaces(): boolean {
+          return true;
+        }
+      }
+
+      schema = await buildSchema({
+        resolvers: [SampleResolver],
+        directives: [testDirective],
+        validate: false,
+      });
+      schema = testDirectiveTransformer(schema);
+    });
+
+    it("should properly emit directive in AST", () => {
+      const multilineInfo = schema.getRootType(OperationTypeNode.QUERY)!.getFields().multiline;
+      const leadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().leadingWhiteSpaces;
+      const multilineAndLeadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().multilineAndLeadingWhiteSpaces;
+      const rawMultilineAndLeadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().rawMultilineAndLeadingWhiteSpaces;
+
+      expect(() => {
+        assertValidDirective(multilineInfo.astNode, "test");
+        assertValidDirective(leadingWhiteSpacesInfo.astNode, "test");
+        assertValidDirective(multilineAndLeadingWhiteSpacesInfo.astNode, "test");
+        assertValidDirective(rawMultilineAndLeadingWhiteSpacesInfo.astNode, "test", {
+          argNonNullDefault: `"argNonNullDefault"`,
+          argNullDefault: `"argNullDefault"`,
+          argNull: `"argNull"`,
+        });
+      }).not.toThrow();
+    });
+
+    it("should properly apply directive mapper", async () => {
+      const multilineInfo = schema.getRootType(OperationTypeNode.QUERY)!.getFields().multiline;
+      const leadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().leadingWhiteSpaces;
+      const multilineAndLeadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().multilineAndLeadingWhiteSpaces;
+      const rawMultilineAndLeadingWhiteSpacesInfo = schema
+        .getRootType(OperationTypeNode.QUERY)!
+        .getFields().rawMultilineAndLeadingWhiteSpaces;
+
+      expect(multilineInfo.extensions).toMatchObject({
+        TypeGraphQL: { isMappedByDirective: true },
+      });
+      expect(leadingWhiteSpacesInfo.extensions).toMatchObject({
+        TypeGraphQL: { isMappedByDirective: true },
+      });
+      expect(multilineAndLeadingWhiteSpacesInfo.extensions).toMatchObject({
+        TypeGraphQL: { isMappedByDirective: true },
+      });
+      expect(rawMultilineAndLeadingWhiteSpacesInfo.extensions).toMatchObject({
+        TypeGraphQL: { isMappedByDirective: true },
+      });
+    });
+  });
+
   describe("errors", () => {
     beforeEach(async () => {
       getMetadataStorage().clear();

diff --git a/tests/helpers/directives/TestDirective.ts b/tests/helpers/directives/TestDirective.ts
@@ -9,9 +9,11 @@ import {
   type GraphQLInputObjectTypeConfig,
   GraphQLInterfaceType,
   type GraphQLInterfaceTypeConfig,
+  GraphQLNonNull,
   GraphQLObjectType,
   type GraphQLObjectTypeConfig,
   type GraphQLSchema,
+  GraphQLString,
 } from "graphql";
 
 function mapConfig<
@@ -36,6 +38,19 @@ function mapConfig<
 
 export const testDirective = new GraphQLDirective({
   name: "test",
+  args: {
+    argNonNullDefault: {
+      type: new GraphQLNonNull(GraphQLString),
+      defaultValue: "argNonNullDefault",
+    },
+    argNullDefault: {
+      type: GraphQLString,
+      defaultValue: "argNullDefault",
+    },
+    argNull: {
+      type: GraphQLString,
+    },
+  },
   locations: [
     DirectiveLocation.OBJECT,
     DirectiveLocation.FIELD_DEFINITION,

diff --git a/tests/helpers/directives/assertValidDirective.ts b/tests/helpers/directives/assertValidDirective.ts
@@ -38,12 +38,17 @@ export function assertValidDirective(
       expect(directive.arguments).toBeFalsy();
     }
   } else {
+    expect(directive.arguments).toHaveLength(Object.keys(args).length);
     expect(directive.arguments).toEqual(
-      Object.keys(args).map(arg => ({
-        kind: "Argument",
-        name: { kind: "Name", value: arg },
-        value: parseValue(args[arg]),
-      })),
+      expect.arrayContaining(
+        Object.keys(args).map(arg =>
+          expect.objectContaining({
+            kind: "Argument",
+            name: expect.objectContaining({ kind: "Name", value: arg }),
+            value: expect.objectContaining(parseValue(args[arg], { noLocation: true })),
+          }),
+        ),
+      ),
     );
   }
 }