dotansimha / graphql-code-generator-community

MIT License
118 stars 155 forks source link

`near-operation-file` does not import scalars, leading to broken code #341

Open rixth opened 1 year ago

rixth commented 1 year ago

Which packages are impacted by your issue?

near-operation-file preset

Describe the bug

When a .gql document contains a query to a field that is a custom scalar, the generated .ts file does not import the scalar, and therefore TypeScript build fails.

Your Example Website or App

https://github.com/rixth/gql-repro

Steps to Reproduce the Bug or Issue

  1. Run npm i && npx graphql-code-generator -v --config codegen.ts
  2. Open src/Query.gql.generated.ts
  3. Observe that MyScalar is underlined red, as it is not resolved or imported, despite scalars: { MyScalar: './ExternalType#MyScalar' } being passed in codegen.ts

Expected behavior

import { MyScalar } from './ExternalType'; should have been added to the top of the file (such as it is in the base types.generated.ts file)

Screenshots or Videos

No response

Platform

Codegen Config File

import { CodegenConfig } from '@graphql-codegen/cli';

const scalars = {
    MyScalar: './ExternalType#MyScalar',
};

const config: CodegenConfig = {
    schema: [
        'src/GqlTypedef.ts',
    ],
    documents: ['src/Query.gql'],
    generates: {
        './src/types.generated.ts': {
            config: {
                skipTypename: true,
                avoidOptionals: true,
                strictScalars: true,
                scalars,
            },
            plugins: ['typescript'],
        },
        './': {
            preset: 'near-operation-file',
            presetConfig: {
                extension: '.gql.generated.ts',
                baseTypesPath: './src/types.generated.ts',
            },
            config: {
                skipTypename: true,
                avoidOptionals: true,
                strictScalars: true,
                scalars,
            },
            plugins: ['typescript-operations'],
        },
    },
};

export default config;

Additional context

updated patch and test in comments

rixth commented 1 year ago

Here is a diff that applies a fix in a better way, along with a test case.

Note that this is missing one thing I wasn't able to figure out: how to not import scalars that are unused. The test case has a commented out assertion for this situation. I'm hoping that someone with more knowledge of how the AST & schema interact can help me out with this part. I think that the isUsingTypes might be part of the solution, but I wasn't able to get the test passing.

diff --git a/packages/presets/near-operation-file/src/resolve-document-imports.ts b/packages/presets/near-operation-file/src/resolve-document-imports.ts
index ba506e771..a2c5e08d4 100644
--- a/packages/presets/near-operation-file/src/resolve-document-imports.ts
+++ b/packages/presets/near-operation-file/src/resolve-document-imports.ts
@@ -64,7 +64,7 @@ export function resolveDocumentImports<T>(
     schemaObject,
     dedupeFragments,
   );
-  const { baseOutputDir, documents } = presetOptions;
+  const { baseOutputDir, documents, config } = presetOptions;
   const { generateFilePath, schemaTypesSource, baseDir, typesImport } = importResolverOptions;

   return documents.map(documentFile => {
@@ -96,6 +96,25 @@ export function resolveDocumentImports<T>(
         importStatements.unshift(schemaTypesImportStatement);
       }

+      Object.entries(config.scalars || {}).forEach(([scalarName, scalarDefinition]: [string, string]) => {
+        // TODO somehow check if the scalar is actually being used in the documentFile.document
+
+        // Scalar definitions not including # are assumed to be globally available
+        if (scalarDefinition.includes('#')) {
+          const [scalarFile, exportedName] = scalarDefinition.split('#');
+          const scalarImportStatement = generateImportStatement({
+            baseDir,
+            emitLegacyCommonJSImports: presetOptions.config.emitLegacyCommonJSImports,
+            importSource: resolveImportSource(scalarFile),
+            baseOutputDir,
+            outputPath: generatedFilePath,
+            typesImport: false,
+          });
+          const importName = (exportedName === scalarName) ? scalarName : `${exportedName} as ${scalarName}`;
+          importStatements.unshift(scalarImportStatement.replace('*', `{ ${importName} }`));
+        }
+      });
+
       return {
         filename: generatedFilePath,
         documents: [documentFile],
diff --git a/packages/presets/near-operation-file/tests/fixtures/issue-341.ts b/packages/presets/near-operation-file/tests/fixtures/issue-341.ts
new file mode 100644
index 000000000..16b81eba1
--- /dev/null
+++ b/packages/presets/near-operation-file/tests/fixtures/issue-341.ts
@@ -0,0 +1,7 @@
+import { gql } from 'graphql-request';
+
+export const A = gql`
+  query A {
+      fieldThatIsCustomScalar
+  }
+`;
diff --git a/packages/presets/near-operation-file/tests/near-operation-file.spec.ts b/packages/presets/near-operation-file/tests/near-operation-file.spec.ts
index e2b5f50f3..8661e69b6 100644
--- a/packages/presets/near-operation-file/tests/near-operation-file.spec.ts
+++ b/packages/presets/near-operation-file/tests/near-operation-file.spec.ts
@@ -591,6 +591,39 @@ describe('near-operation-file preset', () => {
       const imports = parentContent.match(/import.*UserNameFragment/g);
       expect(imports).toHaveLength(1);
     });
+
+    it('#341 - importing scalars', async () => {
+      const result = await executeCodegen({
+        schema: [
+          /* GraphQL */ `
+            scalar CustomScalar
+
+            extend type Query {
+                fieldThatIsCustomScalar: CustomScalar!
+            }
+          `,
+        ],
+        config: {
+          scalars: {
+            CustomScalar: './ExternalTypeFile#CustomScalar',
+            UnusedCustomScalar: './AnotherExternalTypeFile#UnusedCustomScalar',
+          }
+        },
+        documents: [path.join(__dirname, 'fixtures/issue-341.ts')],
+        generates: {
+          'out1.ts': {
+            preset,
+            presetConfig: {
+              baseTypesPath: 'types.ts',
+            },
+            plugins: ['typescript-operations'],
+          },
+        },
+      });
+
+      expect(result[0].content).toMatch(/import { CustomScalar } from '[.\\/]*\/ExternalTypeFile';/);
+      // expect(result[0].content).not.toMatch(`UnusedCustomScalar`);
+    });
   });

   it('should not add imports for fragments in the same location', async () => {
rixth commented 1 year ago

For anyone who runs across this in the future, I found a workaround. It involves passing a different scalar maps to the basetypescript types generation and the near-operation-file generation.

The base type generator gets a config like this:

scalars: {
    PermissionCode: 'modules/Permissions.generated#PermissionCode',
    FeatureName: 'modules/Features.generated#FeatureName',
};

Then, in near-operation-file, since there's already a statement like import * as Types from '../../types.generated'; at the top of each, the scalar map above is transformed at runtime to instead reference this existing import:

scalars: {
    PermissionCode: 'Types.Scalars['PermissionCode']',
    FeatureName: 'Types.Scalars['FeatureName']',
};

Complete configuration:

import { CodegenConfig } from '@graphql-codegen/cli';

const scalars = {
    PermissionCode: 'modules/Permissions.generated#PermissionCode',
    FeatureName: 'modules/Features.generated#FeatureName',
};

const config: CodegenConfig = {
    schema: [/* ... */],
    documents: [/* ... */],
    generates: {
        './src/types.generated.ts': {
            config: {
                /* ... */
                strictScalars: true,
                scalars,
            },
            plugins: ['typescript'],
        },
        './': {
            preset: 'near-operation-file',
            presetConfig: {
                extension: '.gql.generated.ts',
                baseTypesPath: './src/types.generated.ts',
            },
            config: {
                /* ... */
                strictScalars: true,
                scalars: Object.fromEntries(
                    Object.entries(scalars).map(([name]: [string, string]) => [name, `Types.Scalars['${name}']`]),
                ),
            },
            plugins: ['typescript-operations', 'typed-document-node'],
        },
    },
};

export default config;
Rubilmax commented 6 months ago

This indeed is a workaround, but the result file still re-declares Scalars locally, which is sub-optimal:

export type Scalars = {
  CustomScalar: {
    input: Types.Scalars["CustomScalar"];
    output: Types.Scalars["CustomScalar"];
  };
};

We could instead directly reference the Scalars type already defined & exported from types.generated.ts