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

feat: allow using file paths for scalars: config like you can for enumValues #2418

Merged
merged 8 commits into from
Aug 29, 2019
Merged

feat: allow using file paths for scalars: config like you can for enumValues #2418

merged 8 commits into from
Aug 29, 2019

Conversation

ForbesLindesay
Copy link
Contributor

This allows you to do:

config:
  scalars:
    DateTime: Date
    JSON: { [key: string]: any }
    CustomType: ../scalars#CustomType

or even:

config:
  scalars: ../scalars

Forbes Lindesay added 8 commits August 21, 2019 16:19
…enumValues:`

This allows you to do:

```yaml
config:
  scalars:
    DateTime: Date
    JSON: { [key: string]: any }
    CustomType: ../scalars#CustomType
```

or even:

```yaml
config:
  scalars: ../scalars
```

and have the scalars be directly imported from other files
@dotansimha
Copy link
Owner

That's great! Thank you @ForbesLindesay !
Those changes effects some parts of the internal core, and effects almost all plugins. Can you please add some unit tests to make sure those new forms of scalars are generated correctly?

Thanks!

@ForbesLindesay
Copy link
Contributor Author

It took me a pretty long time to make this change relative to what I expected. I don't think I'm going to find time to add tests any time soon.

@ForbesLindesay ForbesLindesay marked this pull request as ready for review August 27, 2019 11:57
@dotansimha
Copy link
Owner

All good @ForbesLindesay , I'll add some tests later today. Thank you!

@dotansimha dotansimha merged commit f525008 into dotansimha:master Aug 29, 2019
@shiatsumat
Copy link

When I use the scalars: [module_path] notation (on ver. 1.7.0), the generator reports Cannot use string scalars mapping when building without a schema;
since I have actually specified the schema and enumValues: [module_path] works fine, probably it is because the custom scalar enumerator for scalars: [module_path] notation is not working right.

@dotansimha
Copy link
Owner

@shiatsumat can you please share your config file or a reproduction of the issue?

@shiatsumat
Copy link

Thank you for reply. My .codegen.yml is as follows.

schema: ./graphql/schema.graphql
documents: ./graphql/usage/*.graphql
generates:
  ./graphql/_gen_/resolve.ts:
    plugins:
      - typescript
      - typescript-resolvers
    config:
      preResolveTypes: true
  ./graphql/_gen_/api.ts:
    plugins:
      - typescript
      - typescript-operations
      - typescript-react-apollo
    config:
      preResolveTypes: true
      withComponent: false
      withHOC: false
      withHooks: true
config:
  scalars: ../scalars
  enumValues: ../enums

The graphql folder consists of the following files.

graphql
├── _gen_
│   ├── api.ts
│   └── resolve.ts
├── scalars.ts
├── schema.graphql
├── usage
│   └── usage.graphql
└── values.ts

@shiatsumat
Copy link

@dotansimha Any ideas...?

@ForbesLindesay
Copy link
Contributor Author

This is because the relevant code doesn't have access to the schema, and this PR was getting huge already. You'll need to add a PR to ensure that the schema is actually passed into the relevant function where that exception is thrown.

@shiatsumat
Copy link

@ForbesLindesay You are absolutely right. I myself cannot find the way to fix the code now though...

@dotansimha
Copy link
Owner

I'll take a look, thanks @ForbesLindesay @shiatsumat

@dotansimha
Copy link
Owner

@shiatsumat @ForbesLindesay
I think I managed to fix this issue, by making sure to always pass the schema to buildScalars method. (#2600)
Can you please try the alpha version? 1.7.1-alpha-8ed4b450.74

@shiatsumat
Copy link

@dotansimha Wow great, it works!!

@dchambers
Copy link

Does anyone know what the contents of scalars.ts needs to look like for this to work?

@dchambers
Copy link

dchambers commented Feb 26, 2022

In case anyone else comes this way looking for the same answer, it turned out that scalars.ts needed to look like this:

export type PositiveInt = number
export type CountryCode = string
export type PostalCode = string
// etc...

GraphQL Code Generator doesn't actually process the file itself however; it merely refers to it in the generated code it creates, with type import statements like this:

import { PositiveInt } from '../scalar-types'
import { CountryCode } from '../scalar-types'
import { PostalCode } from '../scalar-types'
// etc...

In my case, types of all of my scalars were defined using the extensions.codegenScalarType pattern that graphql-scalars adheres to, so I wrote the following generation script (generate-graphql-scalars.ts) so I could use that config rather than redundantly redefine everything:

import { writeFileSync } from 'fs'
import { scalarResolvers } from './src/scalar-resolvers' // NOTE: these are my scalar resolvers as an object, ready to spread into my resolver config within Apollo server

const getKeys = Object.keys as <T extends object>(obj: T) => Array<keyof T>

const generateTypes = getKeys(scalarResolvers)
    .map((name) => `export type ${name} = ${scalarResolvers[name].extensions.codegenScalarType}`)
    .join('\n')

writeFileSync('./src/scalar-types.gen.ts', generateTypes)

The prepack for my library now has the following steps:

  1. Generate the scalar types using generate-graphql-scalars.ts.
  2. Generate the schema types using codegen.
  3. Generate the transpiled code using tsc.

Since generate-graphql-scalars.ts imports TS from my src directory that can not yet be built (it depends on the generated types) I use babel-node to import the subset of TypeScript I need without having to compile the whole thing, as follows:

babel-node --presets @babel/preset-typescript --plugins @babel/plugin-transform-modules-commonjs --extensions '.ts' ./generate-graphql-scalars.ts

Hope that helps someone!

@Urigo
Copy link
Collaborator

Urigo commented Feb 28, 2022

@dchambers do you think we could add something to the docs to make it easier for people?
or even a change in the API?

@dchambers
Copy link

@dchambers do you think we could add something to the docs to make it easier for people? or even a change in the API?

Hi @Urigo 👋 . Yes, absolutely. The current documentation for scalars is as follows:

image

whereas I would ideally hope to see something more like this:

scalars

type: ScalarsMap

Extends or overrides the built-in scalars and custom GraphQL scalars to a custom type.

Usage Examples

Inline Definitions

config:
  scalars:
    DateTime: Date
    JSON: { [key: string]: any }
    CustomType: ../scalars#CustomType

where scalars.ts contains:

export type CustomType = string

External Definitions

config:
  scalars: ../scalars.ts

where scalars.ts contains:

export type DateTime = Date
export type JSON = { [key: string]: any }
export type CustomType = string

@dchambers
Copy link

In regards to the potential for API changes, I believe the ideal outcome would be support for pointing directly at graphql-scalar conformant scalar resolvers, as an alternative means by which scalar types can be retrieved -- particularly since graphql-scalar is now also maintained by The Guild.

For example, the 'External Definitions' section in the example above could be sub-divided into two parts:

External Type Definitions

config:
  scalars: ../scalars.ts

where scalars.ts contains:

export type DateTime = Date
export type JSON = { [key: string]: any }
export type CustomType = string

External Scalar Resolver Definitions

config:
  scalars: ../scalar-resolvers.ts

where scalar-resolvers.ts contains:

import { RegularExpression } from 'graphql-scalars'
export { DateTime, JSON } from 'graphql-scalars'

export const CustomType = new RegularExpression('CustomType', /^ABC$/);

@Urigo
Copy link
Collaborator

Urigo commented Mar 1, 2022

@dchambers that is super helpful, thank you!
Do you want to maybe try to help us work on these things?
We can support and guide you as you make your contributions with anything you might need!

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.

5 participants