Skip to content

Navigation Menu

Sign in
Sign up

Add headers support - #35

Open
StevenLangbroek wants to merge 1 commit into
joarwilk:master from
StevenLangbroek:master
Open

Add headers support #35
StevenLangbroek wants to merge 1 commit into
joarwilk:master from
StevenLangbroek:master

Conversation

@StevenLangbroek

@StevenLangbroek StevenLangbroek commented Sep 6, 2017
edited
Loading

Copy link
Copy Markdown

I'm switching from using a GraphQL based service to my own CMS installation, and its security requirements are different, so I needed custom headers :). Some notes on this PR:

  • I'm assuming Object.assign, array destructuring and arrow function support, so Node 6+. Since current LTS is 6, and it'll soon be Node 8, this seems like a reasonable assumption to me. If you disagree, please let me know and I'll refactor to ES5.
  • The output of --help for README.md is differently sorted than yours... No idea why, wasn't sure whether you'd like to maintain the old order...
  • The changelog appears to be out of date, so I didn't bump versions anywhere.

Furthermore, I think you're reaching the point where it might be good to add some integration-level testing (mocking out the network / fs), and some convention etc... Would you like me to add eslint, editorconfig, prettier and basic set up for more extensive testing?

samhagan and mikestaub reacted with thumbs up emoji
Steven Langbroek

StevenLangbroek commented Sep 6, 2017
edited
Loading

Copy link
Copy Markdown
Author

Hmm... the more I think about this, the more I lean towards just supporting the graphql-config standard. There's a parser available too. Thoughts?

Edit: I'd gladly do the legwork for this.

Copy link
Copy Markdown

This is a feature that we would like also. Either directly via headers flag or via .graphqlrc file. A work around at this point is to run the introspection query yourself, write to file, then pass the json to gql2flow.

Copy link
Copy Markdown
Author

@samhagan if you want i have some time over the weekend, i could whip up a PR for using graphql-config...

samhagan reacted with thumbs up emoji

samhagan commented Sep 25, 2017
edited
Loading

Copy link
Copy Markdown

@StevenLangbroek one thing to consider, not related to the headers, is other configuration this package may need. For example, we're using custom scalar types that by default result in any flow types -- these can be fixed by passing many -t flags, but would be better configured in a .gql2flowrc file that could capture any configuration needs.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /