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

Support schema AST as type definition #300

Merged
merged 8 commits into from Mar 29, 2017

Conversation

Projects
None yet
7 participants
@jamiter
Contributor

jamiter commented Mar 28, 2017

TODO:

  • If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change

The webpack loader and the Meteor build plugin both return a schema AST when importing a .graphql file. It would be great to simply pass this to the makeExecutableSchema function without first having to print it.

This PR makes sure that no needless print and parsing is done when an AST is passed. If an Array is passed it will print every AST it finds so that it can be combined with passed schema strings.

See #273 (comment)

Example:

import schema from './data/schema.graphql'; // No need to print with 'graphql/language/printer'
import resolverMap from './data/resolvers';
import { makeExecutableSchema } from 'graphql-tools';

const executableSchema = makeExecutableSchema({
  typeDefs: schema,
  resolvers: resolverMap,
});

@jamiter jamiter changed the title from Feature/schema ast as type definition to Support schema AST as type definition Mar 28, 2017

jamiter added some commits Mar 28, 2017

Test passing an AST directly
The array test is already covered in the next test
@jamiter

This comment has been minimized.

Contributor

jamiter commented Mar 28, 2017

Ok, coveralls is just awesome 👍

@helfer

This comment has been minimized.

Member

helfer commented Mar 29, 2017

Thanks for the PR @jamiter! There might be a nicer way without the extra print, but I think this is good enough and a smaller change, so I'll merge it! 🙂

@helfer helfer merged commit d78d6f6 into apollographql:master Mar 29, 2017

3 checks passed

CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 99.524%
Details
@jamiter

This comment has been minimized.

Contributor

jamiter commented Mar 29, 2017

Awesome, thanks!

@jaydenseric

This comment has been minimized.

jaydenseric commented Mar 30, 2017

Nice! I wake up every morning with excitement to run yarn outdated to find nice little changes like this. Every day the tooling gets more elegant – thanks.

@jamiter jamiter deleted the jamiter:feature/schema-AST-as-type-definition branch Mar 30, 2017

@rigobcastro

This comment has been minimized.

rigobcastro commented May 13, 2017

Ok I have a question and need help for this, How can I use this on server mode? When I trying to use this great solution I catch the next error:

SyntaxError : /User/user/project/query.graphql: Unexpected token, expected

Looks easy but I don't know how to. Thanks for your support!

@chrisl777

This comment has been minimized.

chrisl777 commented Nov 15, 2017

This fix doesn't appear to be able to handle comments in .graphql files. I see this error when I have a line that is commented out with a # character:

Unexpected character '#'

@jamiter

This comment has been minimized.

Contributor

jamiter commented Nov 15, 2017

@chrisl777, we should be able to add a test to check that. Could you make a PR for it? I'm a bit short on time at the moment.

@Nickersoft

This comment has been minimized.

Nickersoft commented Dec 6, 2017

@jamiter @chrisl777 Any update on this? Does this fix actually work?

@chrisl777

This comment has been minimized.

chrisl777 commented Feb 6, 2018

I have not had any time to check. Anyone else have a chance to play with it?

@ZJUGuoShuai

This comment has been minimized.

ZJUGuoShuai commented Aug 15, 2018

/home/guo/git-repos/artpieces-backend/data/schema.graphql:1
(function (exports, require, module, __filename, __dirname) { scalar Datetime
                                                                     ^^^^^^^^

SyntaxError: Unexpected identifier
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:616:28)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/guo/git-repos/artpieces-backend/dist/app.js:13:15)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:191:16)
    at bootstrap_node.js:612:3

What's wrong with my graphql file? I think nodejs regard it as a js file.

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