Skip to content

feat(flow): add flow types#108

Open
arahansen wants to merge 1 commit intoamount:masterfrom
arahansen:flow-types
Open

feat(flow): add flow types#108
arahansen wants to merge 1 commit intoamount:masterfrom
arahansen:flow-types

Conversation

@arahansen
Copy link

@arahansen arahansen commented Nov 13, 2017

adds flow types that match the nomenclature of flow more closely. fix #103

"@types/node": "^8.0.32",
"coveralls": "^3.0.0",
"jest": "^21.2.1",
"jest-cli": "^21.2.1",
Copy link
Author

Choose a reason for hiding this comment

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

tests were failing for me because jest couldn't find jest-cli

@coveralls
Copy link

coveralls commented Nov 13, 2017

Coverage Status

Coverage increased (+0.09%) to 96.591% when pulling fa64864 on arahansen:flow-types into 458af31 on avantcredit:master.

Copy link
Contributor

@brettjurgens brettjurgens left a comment

Choose a reason for hiding this comment

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

I missed this PR somehow, sorry! I've got a few requests, but otherwise looks awesome and thanks for opening it

"@gql2ts/language-typescript": "^1.3.0",
"@gql2ts/util": "^1.3.0"
"@gql2ts/util": "^1.3.0",
"@types/humps": "^1.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move @types/humps to devDependencies?

export const FLOW_INTERFACE_NAMER: WrapType = name => `${pascalize(name)}`;
export const FLOW_INTERFACE_BUILDER: InterfaceAndTypeBuilder = (name, body) =>
`export type ${name} = ${body}`;
export const FLOW_ENUM_NAME_GENERATOR: WrapType = name => `${pascalize(name)}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add Enum or something to avoid conflicts with the interfaces?

export const FLOW_INTERFACE_BUILDER: InterfaceAndTypeBuilder = (name, body) =>
`export type ${name} = ${body}`;
export const FLOW_ENUM_NAME_GENERATOR: WrapType = name => `${pascalize(name)}`;
export const FLOW_TYPE_PRINTER: TypePrinter = (type, isNonNull) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add Type or something to avoid conflicts with the interfaces/enums? Or just assume that there shouldn't be duplicates. Not sure what the best move is here

Copy link

Choose a reason for hiding this comment

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

I vote no to adding a suffix to avoid conflicts. GraphQL doesn't allow duplicate type names. Enums should always be in SCREMING_CASE. As for conflicts with the IGraphQL* types, they could be separated by moving them to a different file.

Copy link

Choose a reason for hiding this comment

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

Upon further consideration, you seem to be right about having type name conflicts. arahansen/gql2ts#1

Copy link
Contributor

@brettjurgens brettjurgens Feb 2, 2018

Choose a reason for hiding this comment

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

@arahansen i think suffixing this with Type or something similar might be the best way forward.

It's not the best solution, but it works for the typescript version

@0xcaff
Copy link

0xcaff commented Jan 5, 2018

Also closes #78.

@brettjurgens
Copy link
Contributor

@0xcaff @arahansen any update here? would love to get this merged in

@0xcaff
Copy link

0xcaff commented Jan 11, 2018

Blocked on resolving the failure discovered in arahansen/gql2ts#1 .

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.

Why are Graphql Types converted into interfaces ?

4 participants