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

Why are Graphql Types converted into interfaces ? #103

Closed
jwickens opened this issue Oct 28, 2017 · 4 comments · May be fixed by #108
Closed

Why are Graphql Types converted into interfaces ? #103

jwickens opened this issue Oct 28, 2017 · 4 comments · May be fixed by #108

Comments

@jwickens
Copy link

jwickens commented Oct 28, 2017

Here's a snippet from one of my flow exports

flowtypes.js

  export type AnalyticsSetResult = IConversationMetricResult | IBotUpdateMetricResult;

  export interface IAnalyticsSetResult = {
    __typename: "AnalyticsSetResult";
    id: string;
    sets: Array<AnalyticsSet> | null;
}

schema.graphql

interface AnalyticsSetResult {
  id: ID!
  sets: [AnalyticsSet]
}

type ConversationMetricResult implements AnalyticsSetResult {
  id: ID!
  sets: [ConversationMetricSet]
}

I can sort of see why this decision was made - you wanted a concrete type with the name of the interface that is a union of all the types implementing that interface. So to get around that you moved all graphql types to be interfaces and only generate types for these unions. It seems unnecessary and its confusing to have different names between code types and graphql types.

@brettjurgens
Copy link
Contributor

So interfaces are used because the typescript docs recommend using them over type aliases whenever possible (primarily because they're extensible).

Interfaces were chosen before graphql union/interfaces were actually implemented. The type aliases were used for interface/unions, because I found having the union type for all implementations extremely useful.

Is there a downside to using interfaces that I'm unaware of? Perhaps we should look into alternatives for flow if there are

@jwickens
Copy link
Author

jwickens commented Nov 4, 2017

Its not so much the interface, as the conflicting name and the lack of options (at least from what I found) around the union type. For my situation what would be nice:

  • graphql types and interfaces exported to flow interface with original name
  • graphql implementations exported to flow union with name like: interface + Implementation

@arahansen
Copy link

Hey, so I took a stab at building a more "flow-esque" type generation format that matches the nomenclature that flow tends to use. This is the gist of it (using lodash, and node require instead of es modules). If there is interest, I'd be happy to open a PR with these changes to the packages/flow-language settings:

const { camelCase, upperFirst } = require('lodash')

const pascalize = str => upperFirst(camelCase(str))

const { DEFAULT_OPTIONS: TS_OPTIONS } = require('@gql2ts/language-typescript')

const FLOW_WRAP_PARTIAL = partial => `$SHAPE<${partial}>`
const FLOW_INTERFACE_NAMER = name => `${pascalize(name)}`
const FLOW_INTERFACE_BUILDER = (name, body) => `export type ${name} = ${body}`
const FLOW_ENUM_NAME_GENERATOR = name => `${pascalize(name)}`
const FLOW_TYPE_PRINTER = (type, isNonNull) => (isNonNull ? type : `?${type}`)

const FLOW_POST_PROCESSOR = str => `/* @flow */
${str}
`
const FLOW_NAMESPACE_GENERATOR = (namespaceName, interfaces) => `
// graphql flow definitions
${interfaces}
`

const DEFAULT_OPTIONS = {
  ...TS_OPTIONS,
  printType: FLOW_TYPE_PRINTER,
  generateInterfaceName: FLOW_INTERFACE_NAMER,
  generateEnumName: FLOW_ENUM_NAME_GENERATOR,
  interfaceBuilder: FLOW_INTERFACE_BUILDER,
  generateNamespace: FLOW_NAMESPACE_GENERATOR,
  wrapPartial: FLOW_WRAP_PARTIAL,
  postProcessor: FLOW_POST_PROCESSOR,
}

module.exports = DEFAULT_OPTIONS

@brettjurgens
Copy link
Contributor

@arahansen looks good to me, a PR would definitely be welcome 😄

FWIW you can also use pascalize from humps as it's already depended on by @gql2ts/language-typescript

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 a pull request may close this issue.

3 participants