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

Support for GraphQL "extend" keyword #248

Open
louisrli opened this issue Jan 15, 2021 · 6 comments
Open

Support for GraphQL "extend" keyword #248

louisrli opened this issue Jan 15, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@louisrli
Copy link

louisrli commented Jan 15, 2021

Before reporting a bug, please test the beta branch!
(beta branch has no changes to master at the time I'm reporting this)

Bug description

I have a GraphQL schema split into multiple files, also using the extend keyword, which admittedly seems to have pretty shaky support in general. Please let me know if I've made a mistake somehow in setting up Artemis here.

You would see an error for any type on an extended field, such as:

Exception: Field ClassPropertyName(name:r'isReview') was not found in GraphQL type Query.

I've put together a small schema and query below to illustrate this.

Specs

Artemis version: 6.17.3

build.yaml:
# Please paste your `build.yaml` file
targets:
  $default:
    sources:
      - lib/**
    builders:
      artemis:
        options:
          schema_mapping:
            - schema: lib/graphql/generated/testschema/schema.graphql
              queries_glob: lib/graphql/generated/testschema/query.graphql
              output: lib/graphql/generated/graphql_api.graphql.dart
          custom_parser_import: 'package:memm_mobile/graphql/coercers.dart'
          scalar_mapping:
            - graphql_type: Date
              dart_type: DateTime
              use_custom_parser: true
Artemis output:
# Please paste the output of
$ flutter pub run build_runner build --verbose
#or
$ pub run build_runner build --verbose

[  +95 ms] executing: [/Users/Louis/Downloads/flutter/] git -c log.showSignature=false log -n 1 --pretty=format:%H
[  +65 ms] Exit code 0 from: git -c log.showSignature=false log -n 1 --pretty=format:%H
[        ] b0a22998593fc605c723dee8ff4d9315c32cfe2c
[   +1 ms] executing: [/Users/Louis/Downloads/flutter/] git tag --points-at b0a22998593fc605c723dee8ff4d9315c32cfe2c
[ +139 ms] Exit code 0 from: git tag --points-at b0a22998593fc605c723dee8ff4d9315c32cfe2c
[   +1 ms] 1.25.0-8.2.pre
[  +58 ms] executing: [/Users/Louis/Downloads/flutter/] git rev-parse --abbrev-ref --symbolic @{u}
[  +12 ms] Exit code 0 from: git rev-parse --abbrev-ref --symbolic @{u}
[        ] origin/beta
[        ] executing: [/Users/Louis/Downloads/flutter/] git ls-remote --get-url origin
[   +8 ms] Exit code 0 from: git ls-remote --get-url origin
[        ] https://github.com/flutter/flutter.git
[  +62 ms] executing: [/Users/Louis/Downloads/flutter/] git rev-parse --abbrev-ref HEAD
[  +13 ms] Exit code 0 from: git rev-parse --abbrev-ref HEAD
[        ] beta
[   +9 ms] executing: sw_vers -productName
[  +27 ms] Exit code 0 from: sw_vers -productName
[        ] Mac OS X
[        ] executing: sw_vers -productVersion
[  +16 ms] Exit code 0 from: sw_vers -productVersion
[        ] 10.13.6
[        ] executing: sw_vers -buildVersion
[  +18 ms] Exit code 0 from: sw_vers -buildVersion
[        ] 17G66
[   +8 ms] executing: sysctl hw.optional.arm64
[   +6 ms] Exit code 1 from: sysctl hw.optional.arm64
[        ] sysctl: unknown oid 'hw.optional.arm64'
[  +68 ms] Artifact Instance of 'AndroidGenSnapshotArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'AndroidInternalBuildArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IOSEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterWebSdk' is not required, skipping update.
[   +3 ms] Artifact Instance of 'WindowsEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'MacOSEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'LinuxEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'LinuxFuchsiaSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'MacOSFuchsiaSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterRunnerSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterRunnerDebugSymbols' is not required, skipping update.
[  +67 ms] Artifact Instance of 'MaterialFonts' is not required, skipping update.
[        ] Artifact Instance of 'GradleWrapper' is not required, skipping update.
[        ] Artifact Instance of 'AndroidGenSnapshotArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'AndroidInternalBuildArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IOSEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterWebSdk' is not required, skipping update.
[        ] Artifact Instance of 'FlutterSdk' is not required, skipping update.
[        ] Artifact Instance of 'WindowsEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'MacOSEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'LinuxEngineArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'LinuxFuchsiaSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'MacOSFuchsiaSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterRunnerSDKArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FlutterRunnerDebugSymbols' is not required, skipping update.
[        ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'FontSubsetArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'PubDependencies' is not required, skipping update.
[  +34 ms] Using /Users/Louis/Downloads/flutter/.pub-cache for the pub cache.
[        ] executing: /Users/Louis/Downloads/flutter/bin/cache/dart-sdk/bin/pub run build_runner build --verbose
[INFO] Generating build script...
[INFO] Generating build script completed, took 546ms

[WARNING] build.fallback:
The package `memm_mobile` does not include some required sources in any of its targets (see their build.yaml file).
The missing sources are:
  - $package$
[INFO] BuildDefinition:Initializing inputs
[INFO] BuildDefinition:Reading cached asset graph...
[INFO] BuildDefinition:Reading cached asset graph completed, took 97ms

[INFO] BuildDefinition:Checking for updates since last build...
[INFO] BuildDefinition:Checking for updates since last build completed, took 719ms

[INFO] Build:Running build...
[INFO] Build:Running build completed, took 17ms

[INFO] Build:Caching finalized dependency graph...
[INFO] Build:Caching finalized dependency graph completed, took 59ms

[SEVERE] artemis:artemis on lib/$lib$ (cached):

Exception: Field ClassPropertyName(name:r'isReview') was not found in GraphQL type Query.
Make sure your query is correct and your schema is updated.
package:artemis/generator.dart 298:5                  createClassProperty
package:artemis/visitor/generator_visitor.dart 78:22  GeneratorVisitor.visitFieldNode
package:gql/src/ast/ast.dart 213:34                   FieldNode.accept
package:gql/src/ast/ast.dart 10:11                    _visitOne
package:gql/src/ast/ast.dart 17:17                    _visitAll.<fn>
dart:core                                             List.forEach
package:gql/src/ast/ast.dart 16:12                    _visitAll
package:gql/src/ast/ast.dart 34:13                    Node.visitChildren.<fn>
dart:core                                             List.forEach
package:gql/src/ast/ast.dart 31:52                    Node.visitChildren
package:gql/src/ast/visitor.dart 724:12               RecursiveVisitor.visitSelectionSetNode
package:artemis/visitor/generator_visitor.dart 36:11  GeneratorVisitor.visitSelectionSetNode
package:gql/src/ast/ast.dart 177:34                   SelectionSetNode.accept
package:gql/src/ast/ast.dart 10:11                    _visitOne
package:gql/src/ast/ast.dart 36:13                    Node.visitChildren.<fn>
dart:core                                             List.forEach
package:gql/src/ast/ast.dart 31:52                    Node.visitChildren
package:gql/src/ast/visitor.dart 688:12               RecursiveVisitor.visitOperationDefinitionNode
package:gql/src/ast/ast.dart 156:34                   OperationDefinitionNode.accept
package:gql/src/ast/ast.dart 10:11                    _visitOne
package:gql/src/ast/ast.dart 17:17                    _visitAll.<fn>
dart:core                                             List.forEach
package:gql/src/ast/ast.dart 16:12                    _visitAll
package:gql/src/ast/ast.dart 34:13                    Node.visitChildren.<fn>
dart:core                                             List.forEach
package:gql/src/ast/ast.dart 31:52                    Node.visitChildren
package:gql/src/ast/visitor.dart 532:12               RecursiveVisitor.visitDocumentNode
package:gql/src/ast/ast.dart 75:34                    DocumentNode.accept
package:artemis/generator.dart 225:7                  generateDefinitions.<fn>
dart:core                                             Iterable.toList
package:artemis/generator.dart 67:8                   generateLibrary
package:artemis/builder.dart 137:29                   GraphQLQueryBuilder.build

[SEVERE] Build:
Failed after 124ms
[+4169 ms] "flutter run" took 4,294ms.
[ +256 ms] ensureAnalyticsSent: 253ms
[   +2 ms] Running shutdown hooks
[        ] Shutdown hooks complete
[        ] exiting with code 1
GraphQL schema:
# a minimum reproducible schema of the bug.
type Query {
  loggedInUser: User
}

type Mutation {
  updatePreferences(preferences: UpdatePreferencesInput!): User!
}

input UpdatePreferencesInput {
  id: ID!
}

type User {
  id: ID!
  email: String!
  createdTimestamp: Date!
}

extend type Query {
  isReview: Boolean!
}

GraphQL query:
# If possible, please paste your GraphQL query file,
# or a minimum reproducible query of the bug.
query isReview {
  isReview
}
@louisrli louisrli added the bug Something isn't working label Jan 15, 2021
@comigor comigor added enhancement New feature or request and removed bug Something isn't working labels Jan 15, 2021
@comigor
Copy link
Owner

comigor commented Jan 15, 2021

Hello @louisrli!

Yes, I'm aware extensions are an integral part of GraphQLspec. However, they are barely used, as one could just add a new field on the original type (unless, and that may be your case, you're doing some kind of schema stitching).

Given this lack of general usage (you're the first person who even talked about it), I've not prioritized it yet.

If you're interested on it, I'd appreciate if you could open a PR implementing extensions (at least for your use case), so the library would grow!

@louisrli
Copy link
Author

louisrli commented Jan 15, 2021

Thanks for the really fast response. Yes, I didn't want to add one schema mapping for each of my schemas (I'm not sure that would work either because the definitions are shared/overlapped among the schema files)

For background, I've split my GraphQL files because the schema is pretty large (just for organization). In my JS server, it looks something like this, so it's merging them:

const typesArray = loadFilesSync('./src/schema', { extensions: ['graphql'] });

const server = new GraphQLServer({
  typeDefs: mergeTypeDefs(typesArray),
  ...
});

When attempting to use Artemis, I was trying to do some type of "stitching" like so:

cat admin.graphql schema.graphql search.graphql signup.graphql tutor.graphql > combined.graphql

Then in my build.yaml:

- schema: lib/graphql/generated/schema/combined.graphql

I'd agree that it's not worth your time to make it a priority. I think that it's possible to have a workaround, I will write a script that does something similar to what my server does, then try to see if it can reprint the GraphQL types to "stitch" them together. If I make it work, I'll write a solution here for anyone else who runs into the same thing :)

@louisrli
Copy link
Author

I think something like this (but requiring use of a JS script outside of Flutter) would work: https://www.graphql-tools.com/docs/schema-merging/#print-merged-typedefs

@comigor
Copy link
Owner

comigor commented Jan 15, 2021

If implemented in Dart, this "script" could be the start of making Artemis support it!

Maybe some kind of pre-process function (based on some new configuration on Artemis) that would join schemas, stitch them together (expanding extensions on their respective types), and use the the result into Artemis as if it was the original schema.

@louisrli
Copy link
Author

@comigor I'm unsure how hard it is to join schemas in Dart, it depends on what's currently available in the GQL package. The script I ended up has most of its heavy lifting done by the JavaScript package @graphql-tools/merge, which seems to take care of a lot of stuff relating normalizing the AST of the merged schema and such (from a quick skim of their code): https://github.com/ardatan/graphql-tools/tree/master/packages/merge/src

I imagine merging GraphQL schemas is a somewhat non-trivial task, but I wonder if the Dart gql package has a similar functionality that could make it easier here.

But don't take my word for it, I don't know the landscape at all. To be honest, I've written more YAML than Dart at this current point in time...but hopefully that'll change over the next few weeks when I start writing this app ;)


For those who may check this and are willing to pop a Typescript/JS script somewhere, this is what I ended up using.

import { mergeTypeDefs } from '@graphql-tools/merge';
import { loadFilesSync } from '@graphql-tools/load-files';
import { print } from 'graphql';
import * as fs from 'fs';

const typesArray = loadFilesSync('./src/schema', { extensions: ['graphql'] });
const merged = mergeTypeDefs(typesArray);
const printedTypeDefs = print(merged);
fs.writeFileSync('joined.graphql', printedTypeDefs);

then I'll write a bash script to move it from the server folder into the mobile app folder as part of the generation process.

@louisrli
Copy link
Author

louisrli commented Jan 15, 2021

Also, I'm not even sure if the extend keyword is supposed to be used in the same file as the original type (I think it's not). So this whole issue might be an artifact of me attempting to cat the files together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants