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

Consider package imports. #57

Closed
wtgtybhertgeghgtwtg opened this issue Jan 25, 2018 · 2 comments · Fixed by #216
Closed

Consider package imports. #57

wtgtybhertgeghgtwtg opened this issue Jan 25, 2018 · 2 comments · Fixed by #216

Comments

@wtgtybhertgeghgtwtg
Copy link

Consider the example from graphql-binding-github.

# import Repository from "./github.graphql"

type Query {
  hello(name: String): String!
  favoriteRepos: [Repository!]!
}

It requires an up to date github.graphql, which is generated, to exist next to app.graphql, which is not. github.graphql also has to be put into version control and manually regenerated whenever the GitHub GraphQL endpoint is updated.
However, graphql-binding-github exports a github.graphql. Using that, I wouldn't have to put it in version control or regenerate it, just update graphql-binding-github. But, rewriting it that way comes with another problem.

# import Repository from "../node_modules/graphql-binding-github/schema/schema.graphql"

Things get even hairier if app.graphql is deeply nested or in a monorepo.
Ideally, I should be able to do something like

# import Repository from "graphql-binding-github/schema/schema.graphql"

To take it a step further, you could default to a top-level schema.graphql or use a graphql:main field in the package.json and do something like

# import Repository from "graphql-binding-github"

I believe this is currently possible with Object Imports, but you'd have to do something like

const githubSchema = fs.readSync('Path to schema.graphql');
importSchema(mySchema, {'graphql-binding-github': githubSchema});
@schickling
Copy link
Contributor

I remember @kbrandwijk working on something very similar?

@lfades
Copy link

lfades commented May 4, 2018

Oh I just saw this issue its related to this PR, but it's never too late to comment!

SpaceK33z added a commit that referenced this issue Sep 7, 2018
This is a continuation of PR #136 from @lfades (spoke to @lfades about this and he was okay with it).

It uses `resolve-from` instead of `require.resolve` with the `paths` option because it is not compatible with Node < 8.

Also I think this one works a bit differently; it doesn't introduce a breaking change since it will first try to look up the path relative to the file the import is in, and only if that fails it will use `resolve-from`.

Fixes #57
SpaceK33z added a commit that referenced this issue Sep 7, 2018
This is a continuation of PR #136 from @lfades (spoke to @lfades about this and he was okay with it).

It uses `resolve-from` instead of `require.resolve` with the `paths` option because it is not compatible with Node < 8.

Also I think this one works a bit differently; it doesn't introduce a breaking change since it will first try to look up the path relative to the file the import is in, and only if that fails it will use `resolve-from`.

Fixes #57
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 a pull request may close this issue.

3 participants