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

schema:download generates incompatible schema JSON #555

Closed
1 task done
michaelbromley opened this issue Aug 17, 2018 · 8 comments · Fixed by #562
Closed
1 task done

schema:download generates incompatible schema JSON #555

michaelbromley opened this issue Aug 17, 2018 · 8 comments · Fixed by #562
Labels
🌹 has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository

Comments

@michaelbromley
Copy link

michaelbromley commented Aug 17, 2018

  • has-reproduction

Version

apollo v1.7.0

Description

Running apollo schema:download generates JSON which omits the "__schema" root property. This seems to break with convention (e.g. intellij GraphQL plugin, graphql-js introspection tests, relay-starter-kit, spectrum).

I am using the intellij GraphQL plugin and it is unfortunately unable to read the output from apollo at the moment.

Steps to reproduce

  1. npm install apollo
  2. npx apollo schema:download schema.json --endpoint=https://graphql-pokemon.now.sh

Expected

The schema.json file should have the root property "__schema"

Actual

The"__schema" property is missing and instead it seems to have been unwrapped so that the root properties are "queryType", "mutationType", etc.

@ghost ghost added the 🌹 has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository label Aug 17, 2018
@michaelbromley michaelbromley changed the title schema:download generates invalid schema JSON schema:download generates incompatible schema JSON Aug 17, 2018
@ghost ghost added the 🌹 has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository label Aug 17, 2018
@jamesmbourne
Copy link
Contributor

This schema format is also incompatible with apollo-android, which is still internally using the now-deprecated apollo-codegen.

Quick-and-dirty sed to wrap the existing file:

sed -i 's/.*/{"__schema":&}/' schema.json

@danilobuerger
Copy link
Contributor

Changing this would also mean changing codegen:generate as this relies on the schema without __schema.

@jamesmbourne
Copy link
Contributor

@danilobuerger I am currently using codegen:generate with TypeScript and it appears to have no issue parsing a schema with __schema as the root node, but I haven't tried this with any other languages supported by generate.

The https://github.com/graphql/graphql-js reference implementation used here makes reference to __schema in introspectionQuery.js#L22 so I think this shouldn't be too much of an issue.

I believe that it may in fact be the case that the root node should be data, with __schema as a child of that based upon this schema.json, although I can't find any kind of specification to confirm this.

@jamesmbourne
Copy link
Contributor

jamesmbourne commented Aug 28, 2018

I've resolved the issue discussed here in #562, although I've also noticed that codegen:generate also outputs a schema missing this nesting when using the JSON output format.

Is this something that should be included in the aforementioned PR?

@martijnwalraven
Copy link
Contributor

martijnwalraven commented Aug 29, 2018

@jamesmbourne I merged your PR and that seems like the right fix, but wondering what you mean by:

I've also noticed that codegen:generate also outputs a schema missing this nesting when using the JSON output format

@jamesmbourne
Copy link
Contributor

@martijnwalraven Thanks for getting that merged in!

I was referring to the codegen:generate when used with an output type of JSON (see generate.ts#L136 for the implementation and here for the covering test case and the snapshot), although I think this is behaving as expected by outputting "operations" as the root key, and doesn't require the "data"/"__schema" keys so that point can be ignored.

martijnwalraven added a commit that referenced this issue Aug 30, 2018
In contrast to #562, this no longer nests the introspection result under `data`, although it does preserve `__schema`.

Also see #555.
@martijnwalraven
Copy link
Contributor

martijnwalraven commented Aug 30, 2018

@jamesmbourne After some discussion, I pushed another change to no longer nest the introspection result under data in the downloaded schema.json (but keeping __schema).

This seems to make more semantic sense, because you'd never want to store a schema with errors, and this is also what buildClientSchema expects.

@dariuxmx
Copy link

dariuxmx commented Aug 30, 2018

Seems happen with iOS too using apollo schema:download.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌹 has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository
Projects
None yet
5 participants