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

sort documents and fragments #384

Closed
micimize opened this issue Jun 18, 2018 · 9 comments
Closed

sort documents and fragments #384

micimize opened this issue Jun 18, 2018 · 9 comments

Comments

@micimize
Copy link
Contributor

Ran into an issue where a-fragment.gql depended on b-fragment.gql, which throws an error when the generated output is actual code (i.e. const B = a.Fragment, although interestingly only at runtime when namespaced). Although an ideal solution might be to add a flag for sorting based on #imports or something, it's probably more expedient to add something like a files option to the config that has some globbing syntax:

{
  // dedup and order a file by first match
  "files":  ["src/a-fragment.gql", "src/*.gql"]
}
@dotansimha
Copy link
Owner

Hmm, so the issue happens on client-side documents, that you build using JavaScript variables? and then it uses fragments from another file? can you please give an example for this usage?
I think we it might be possible to read the documents files as JavaScript file instead of text files, and then it will resolve and require the other files automatically.

@dotansimha dotansimha added the bug label Jun 25, 2018
@micimize
Copy link
Contributor Author

While writing this I realized I can probably work around my specific issue with var hoisting, but I'll still provide the details as it might be an issue for languages where hoisting isn't allowed:

I'm using the #import graphql convention I linked to above, so I don't think loading via js would work. The app uses graphql-tag/loader to import .gql files into js. We generate the correct types, it's just they're out of order. Here's a simplified example:

# src/event.gql
fragment eventFields on Event {
  id
  start
  end
  title
  description
}
# src/calendar.gql
#import "./event.gql"
fragment calendarEvents on Calendar {
  events: eventsByParentCalendarId {
    nodes {
      ...eventFields
    }
  }
}

This will output a type file with calendarEvents before eventFields, which is invalid without hoisting (but only at runtime)

@lucasavila00
Copy link
Contributor

You should use https://github.com/prismagraphql/graphql-import to use this syntax and import the file correctly.

@dotansimha
Copy link
Owner

I'm not that familiar with the #import syntax, but I think we can add support for that, because at the moment it just treated as comment. @degroote22 @micimize what do you think?

@lucasavila00
Copy link
Contributor

lucasavila00 commented Jul 3, 2018

It's not official, it's still a spec proposal.
Given that and the fact that the implementation is quite stable and well supported, it's ok to add it.

What I don't like is the fact that people should be educated to use this library properly, and if they don't they will come here writing issues. I think this should stay at userspace while graphql-code-generator follows whats in the spec.

It wouldn't hurt to add a section to the Readme or create another file called "recipes" or something like this.
Then we'd put a section there called "splitting your schema across multiple files / schema stitching" and explain how to do it. It's simple.
I'm currently using a script to import the file that goes like this:

import { importSchema } from "graphql-import";
export default importSchema("./src/schema.graphql");

and invoking graphql-code-generator like this:

gql-gen --require ts-node/register --template typescript --export ./scripts/importSchema.ts --out ./src/generated/

Who needs it could do it by themselves, imho.

@micimize
Copy link
Contributor Author

micimize commented Jul 3, 2018

@degroote22 graphql-import seems to be specifically for schemas, and not for operation documents. Also, I don't think we have a way of passing a module as the documents flag.

But, maybe that's exactly what we should implement here - provide a flag like --export-documents ./src/client-queries.ts for the use to hook into and add whatever middleware they want.

@lucasavila00
Copy link
Contributor

What's the problem with the current approach of having the CLI look for the documents?
I guess I didn't understand what you said, if you wouldn't mind explaining it better...

@micimize
Copy link
Contributor Author

micimize commented Jul 4, 2018

@degroote22 in order to apply preprocessing, such as import resolution, like you did on the schema side. Something like

import { resolveDocuments } from "graphql-document-import";
export default resolveDocuments("./src/queries.graphql");

@micimize
Copy link
Contributor Author

Realized this can actually be solved by ordering the arguments to the generator. It would be nice to dedupe the expanded glob, because it seems gql-gen ... src/b/*.gql src/**/*.gql results in the definitions of src/b/*.gql being generated twice. Just a nice-to-have though

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

No branches or pull requests

3 participants