Skip to content

Allow begining pipe in union types #897

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

Closed
alexandrebodin opened this issue Jun 6, 2017 · 5 comments
Closed

Allow begining pipe in union types #897

alexandrebodin opened this issue Jun 6, 2017 · 5 comments
Assignees
Labels

Comments

@alexandrebodin
Copy link
Contributor

Hi

Everything was said in the titile. I would be great to be able to write:

union unionType = 
  | TypeA
  | Type B
  | Type C

instead of

union unionType = 
    TypeA
  | Type B
  | Type C

This would be great for prettier / graphql to make cleaner formatting. And overall go the same wayas Flow and Elm for example :)

@robzhu robzhu self-assigned this Jun 13, 2017
@robzhu
Copy link
Contributor

robzhu commented Jun 13, 2017

Hey @alexandrebodin, thanks for filing this issue. The proposal reasonable and we should be able to get it added soon.

@alexandrebodin
Copy link
Contributor Author

@robzhu If you can give me some pointers I'm down for doing a PR

@robzhu
Copy link
Contributor

robzhu commented Jun 13, 2017

@alexandrebodin awesome, thanks for volunteering =). Start with schema-parser-test.js:460. It has an example of parsing a union type from graphql schema format. If you trace through that, you'll probably find yourself at language/parser.js: parseUnionTypeDefinition. My general advice is to write the failing test first and work backward. I can follow up on your PR with the related spec text changes. Take a look and let me know if you still want to commit to doing this work.

@alexandrebodin
Copy link
Contributor Author

alexandrebodin commented Jun 13, 2017

@robzhu

PR done here: #907
(also opened a PR on the spec graphql/graphql-spec#320)

@robzhu
Copy link
Contributor

robzhu commented Jun 14, 2017

Thank you for your contributions! Both PRs have been merged, closing the issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants