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

Change over to required gql tag #168

Merged
merged 10 commits into from
May 4, 2016
Merged

Change over to required gql tag #168

merged 10 commits into from
May 4, 2016

Conversation

stubailo
Copy link
Contributor

@stubailo stubailo commented May 3, 2016

This will make it required to wrap all queries with:

gql`...`

See docs PR for details: https://github.com/apollostack/docs/pull/63

TODO:

  • Test that appropriate warnings are printed
  • Update change log
  • Export gql from a subdirectory
  • Use npm link to test from an app
  • Set up PR to docs

@stubailo stubailo changed the title [WIP] Change over to required gql tag Change over to required gql tag May 3, 2016
@jbaxleyiii
Copy link
Contributor

Use npm link to test from an app

I haven't tested that yet and it probably won't work? Still need to figure this one out

@stubailo
Copy link
Contributor Author

stubailo commented May 3, 2016

It will work from this branch though since it's before your other changes! Definitely have used that before.

@jbaxleyiii
Copy link
Contributor

Oh cool! If that is the case it should keep working. The deploy branch doesn't change how it works locally, only moves everything to root on publish

@stubailo
Copy link
Contributor Author

stubailo commented May 3, 2016

Yeah I suppose the import paths will be different for npm link though which is a bit odd... we'll figure it out later.

@stubailo
Copy link
Contributor Author

stubailo commented May 3, 2016

Does the PR look good?


assert.throws(() => {
queryManager.query({
// Bamboozle TypeScript into letting us do this
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the trowing tests should be written in JS to avoid this kind of type system coercion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meh it's just one place right? I think it's great that typescript has a way of overriding types to allow us to test an edge case like this.

@stubailo
Copy link
Contributor Author

stubailo commented May 4, 2016

@jbaxleyiii
Copy link
Contributor

@stubailo really excited for this. It looks great to me!

@stubailo
Copy link
Contributor Author

stubailo commented May 4, 2016

Going for it!

@stubailo stubailo merged commit 34ece5c into master May 4, 2016
@jbaxleyiii jbaxleyiii deleted the gql branch May 17, 2016 02:05
jbaxleyiii pushed a commit that referenced this pull request Oct 17, 2017
install graphql for Meteor integration

Right now, you also have to specify `apollo-server@^0.1.1`, but I think [that will change soon](apollographql/meteor-integration#26).
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants