Skip to content

Do not create globalTypes.ts when it is empty#2163

Open
paulmelnikow wants to merge 5 commits intoapollographql:masterfrom
curvewise-forks:ts-empty-global-types
Open

Do not create globalTypes.ts when it is empty#2163
paulmelnikow wants to merge 5 commits intoapollographql:masterfrom
curvewise-forks:ts-empty-global-types

Conversation

@paulmelnikow
Copy link
Copy Markdown

@paulmelnikow paulmelnikow commented Nov 13, 2020

Fix #2080

Alternative to #2055

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

@paulmelnikow
Copy link
Copy Markdown
Author

The Node 8 and 10 builds are failing with TypeErrors that are generated somewhere in the config system. I can't reproduce these locally (even when using Node 8 or 10). Any idea what might be causing these?

    TypeError [ERR_INVALID_ARG_TYPE]: The "warning" argument must be one of type Error or string

      at Config.warn (../../node_modules/@oclif/config/lib/config.js:310:21)
      at Promise.all.map (../../node_modules/@oclif/config/lib/config.js:135:26)

@natterstefan
Copy link
Copy Markdown

Hi 👋,

is there any chance (@maintainers) that we can take a look at this issue?

It would also help if the file just has an empty export export {} in that case (would fix #1179).

@EyMaddis
Copy link
Copy Markdown

EyMaddis commented Feb 11, 2021

I tried it on my project and it did not work.
Custom scalars will end up in context.typesUsed, but won't be in the globals file but instead will be handled by --customScalarsPrefix, for me there is a Guid scalar I define as string

EDIT: I think this is a simple fix:

import { isScalarType } from 'graphql'
...
const usedTypes = context.typesUsed.filter(t => !isScalarType(t))
            if (usedTypes.length > 0) {
....

Just to be clear: I am not a maintainer. Maybe @abernix can help :)

Comment thread packages/apollo/src/generate.ts Outdated
output: generatedGlobalFile.fileContents
};

if (context.typesUsed.length > 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.typesUsed.filter(t => !isScalarType(t)).length > 0
to avoid custom scalar types

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to make this change, though it should probably have a test. What's the behavior you're looking for, exactly?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom scalars like (in schema terms): scalar MyCustomScalar are inside this array of typesUsed. However, these do not generate anything in the globals files since the generator does can not know what the data structure is actually. This would still leading to an empty file.

https://www.apollographql.com/docs/apollo-server/schema/custom-scalars/

The test scenario would require a custom scalar type and a query that does not use any enums or other globals, but returns the custom scalar.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I added the check in 6d42e81.

@macrozone
Copy link
Copy Markdown

its important that this gets merged, it leads to completily pointless problems that could easily be avoided.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code generator should not generate empty global-types

4 participants