facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.42k stars 1.83k forks source link

ERROR: Invalid AST Node when following Step-by-step Guide #3622

Closed skleeschulte closed 3 years ago

skleeschulte commented 3 years ago

I followed the Step-by-step Guide for v12.0.0 using the yarn-commands. Everything works fine, except when executing the very last command - yarn start in step 5 - I get this error output:

node ➜ /workspaces/relay-test (master ✗) $ yarn start
yarn run v1.22.5
$ yarn relay && react-scripts start
$ yarn run relay-compiler --schema schema.graphql --src ./src/ --watchman false $@
$ /workspaces/relay-test/node_modules/.bin/relay-compiler --schema schema.graphql --src ./src/ --watchman false

Writing js
ERROR:
Invalid AST Node: { kind: "Root", operation: "query", loc: { kind: "Source", start: 3, end: 105, source: [Source] }, metadata: null, name: "AppRepositoryNameQuery", argumentDefinitions: [], directives: [], selections: [[Object]], type: Query }.
error Command failed with exit code 100.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 100.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 100.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I double- and triple-checked all steps, verified the schema is saved in UTF-8, stripped out everything from the App.js file except the graphql query, ran yarn run relay-compiler directly, successfully tested the query string with the GitHub GraphQL API Explorer, added and removed whitespace in the query, tested other queries composed with the GitHub GraphQL API Explorer, modified the name of the query, changed App.js to export default App; instead of AppRoot, downgraded the related packages from ^12.0.0 to ^11.0.0, but all to no avail. The error remains:

ERROR: Invalid AST Node: { kind: "Root", operation: "query", loc: { kind: "Source", start: 3, end: 105, source: [Source] }, metadata: null, name: "AppRepositoryNameQuery", argumentDefinitions: [], directives: [], selections: [[Object]], type: Query }.

skleeschulte commented 3 years ago

Ok, I finally found the error: relay-compiler has a peer-dependency of graphql@^15.0.0. However following the step-by-step guide, with yarn add --dev relay-compiler graphql babel-plugin-relay graphql version 16.0.1 is installed.

The step-by-step guide should be updated to include:

# NPM Users
npm install --save relay-runtime react-relay
npm install --save-dev relay-compiler graphql@^15.0.0 babel-plugin-relay

# Yarn Users
yarn add relay-runtime react-relay
yarn add --dev relay-compiler graphql@^15.0.0 babel-plugin-relay
asterikx commented 3 years ago

Couldn't we just upgrade the peer dependency of relay-compiler and babel-plugin-relay to graphql@^16.0.0?

skleeschulte commented 3 years ago

As the error only occurs with graphql v16, and installing graphql v15 fixes the error, I assume that graphql v16 has some breaking changes that would need to be addressed in relay-compiler and babel-plugin-relay before upgrading the peer dependency. So if this won't happen anytime soon, I suggest to update the documentation. BTW, this also applies to the Installation and Setup page.

alunyov commented 3 years ago

We're currently in the process of deprecating current JS compiler: https://github.com/facebook/relay/issues/3180. The new version won't have a dependency on the graphql-js so we should not have that problem there.

skleeschulte commented 3 years ago

Just be aware that the current instructions in the docs are not working, which can be quite frustrating and time-consuming for relay newbies like me, especially as the resulting error message is not very helpful. A peer-dependency-not-met-warning is easily overlooked / not given attention to when following instructions from the docs. Although graphql v16 was released only a few days ago, I'm not the only one who stumbled upon this.

AndrewIngram commented 3 years ago

@alunyov I'm a little confused as to the action here. The doc fixes are being closed without merging because the conclusion is to fix the root cause rather than the docs; but the resolution in this ticket is that it's essentially a "wontfix" because the JS compiler is deprecated -- even though its replacement isn't ready yet.

This feels like a misstep, this issue has come up multiple times in multiple places in the few days since GraphQL 16 has been released -- everyone following the Relay docs today will face this problem. This doesn't leave the open source users of Relay in a good place.

alunyov commented 3 years ago

@skleeschulte @AndrewIngram thank you for your feedback!

As far as I understand, today, if you run:

npm install --save-dev relay-compiler graphql babel-plugin-relay

The output will be something like this:

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN babel-plugin-relay@12.0.0 requires a peer of graphql@^15.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN relay-compiler@12.0.0 requires a peer of graphql@^15.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN test-relay@1.0.0 No description
npm WARN test-relay@1.0.0 No repository field.

So, the npm clearly warns the user that some packages need graphql@^15.0.0 to work correctly.

We'll need to investigate what changes we need to make to the current JS compiler to support the new GraphQL-js version.

skleeschulte commented 3 years ago

Unfortunately, when using yarn the warning is less clear, as it quickly scrolls out of view:

$ yarn add --dev relay-compiler graphql babel-plugin-relay
yarn add v1.22.5
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > react-relay@12.0.0" has unmet peer dependency "react@^16.9.0 || ^17".
warning " > relay-compiler@12.0.0" has incorrect peer dependency "graphql@^15.0.0".
warning " > babel-plugin-relay@12.0.0" has incorrect peer dependency "graphql@^15.0.0".
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 119 new dependencies.
info Direct dependencies
├─ babel-plugin-relay@12.0.0
├─ graphql@16.0.1
└─ relay-compiler@12.0.0
info All dependencies
├─ @babel/code-frame@7.16.0
├─ @babel/core@7.16.0
├─ @babel/helper-create-class-features-plugin@7.16.0
├─ @babel/helper-get-function-arity@7.16.0
├─ @babel/helper-hoist-variables@7.16.0
├─ @babel/helper-skip-transparent-expression-wrappers@7.16.0
├─ @babel/helper-validator-option@7.14.5
├─ @babel/helpers@7.16.0
├─ @babel/highlight@7.16.0
├─ @babel/plugin-proposal-class-properties@7.16.0
├─ @babel/plugin-proposal-object-rest-spread@7.16.0
├─ @babel/plugin-syntax-class-properties@7.12.13
├─ @babel/plugin-syntax-flow@7.16.0
├─ @babel/plugin-syntax-jsx@7.16.0
├─ @babel/plugin-syntax-object-rest-spread@7.8.3
├─ @babel/plugin-transform-arrow-functions@7.16.0
├─ @babel/plugin-transform-block-scoped-functions@7.16.0
├─ @babel/plugin-transform-block-scoping@7.16.0
├─ @babel/plugin-transform-classes@7.16.0
├─ @babel/plugin-transform-computed-properties@7.16.0
├─ @babel/plugin-transform-destructuring@7.16.0
├─ @babel/plugin-transform-flow-strip-types@7.16.0
├─ @babel/plugin-transform-for-of@7.16.0
├─ @babel/plugin-transform-function-name@7.16.0
├─ @babel/plugin-transform-literals@7.16.0
├─ @babel/plugin-transform-member-expression-literals@7.16.0
├─ @babel/plugin-transform-modules-commonjs@7.16.0
├─ @babel/plugin-transform-object-super@7.16.0
├─ @babel/plugin-transform-parameters@7.16.0
├─ @babel/plugin-transform-property-literals@7.16.0
├─ @babel/plugin-transform-react-display-name@7.16.0
├─ @babel/plugin-transform-react-jsx@7.16.0
├─ @babel/plugin-transform-shorthand-properties@7.16.0
├─ @babel/plugin-transform-spread@7.16.0
├─ @babel/plugin-transform-template-literals@7.16.0
├─ @types/parse-json@4.0.0
├─ ansi-regex@5.0.1
├─ ansi-styles@4.3.0
├─ babel-plugin-dynamic-import-node@2.3.3
├─ babel-plugin-macros@2.8.0
├─ babel-plugin-relay@12.0.0
├─ babel-plugin-syntax-trailing-function-commas@7.0.0-beta.0
├─ babel-preset-fbjs@3.4.0
├─ balanced-match@1.0.2
├─ brace-expansion@1.1.11
├─ browserslist@4.17.6
├─ bser@2.1.1
├─ call-bind@1.0.2
├─ callsites@3.1.0
├─ camelcase@5.3.1
├─ caniuse-lite@1.0.30001277
├─ chalk@4.1.2
├─ cliui@6.0.0
├─ color-convert@2.0.1
├─ color-name@1.1.4
├─ concat-map@0.0.1
├─ convert-source-map@1.8.0
├─ cosmiconfig@6.0.0
├─ define-properties@1.1.3
├─ electron-to-chromium@1.3.888
├─ emoji-regex@8.0.0
├─ error-ex@1.3.2
├─ escalade@3.1.1
├─ escape-string-regexp@1.0.5
├─ fb-watchman@2.0.1
├─ find-up@4.1.0
├─ fs.realpath@1.0.0
├─ gensync@1.0.0-beta.2
├─ get-caller-file@2.0.5
├─ get-intrinsic@1.1.1
├─ glob@7.2.0
├─ graphql@16.0.1
├─ has-flag@4.0.0
├─ immutable@3.7.6
├─ import-fresh@3.3.0
├─ inflight@1.0.6
├─ inherits@2.0.4
├─ is-arrayish@0.2.1
├─ is-core-module@2.8.0
├─ is-fullwidth-code-point@3.0.0
├─ jsesc@2.5.2
├─ json-parse-even-better-errors@2.3.1
├─ json5@2.2.0
├─ lines-and-columns@1.1.6
├─ locate-path@5.0.0
├─ minimatch@3.0.4
├─ minimist@1.2.5
├─ ms@2.1.2
├─ node-int64@0.4.0
├─ node-releases@2.0.1
├─ object-keys@1.1.1
├─ object.assign@4.1.2
├─ p-limit@2.3.0
├─ p-locate@4.1.0
├─ p-try@2.2.0
├─ parent-module@1.0.1
├─ parse-json@5.2.0
├─ path-exists@4.0.0
├─ path-is-absolute@1.0.1
├─ path-parse@1.0.7
├─ path-type@4.0.0
├─ picocolors@1.0.0
├─ relay-compiler@12.0.0
├─ require-directory@2.1.1
├─ require-main-filename@2.0.0
├─ resolve-from@4.0.0
├─ resolve@1.20.0
├─ safe-buffer@5.1.2
├─ set-blocking@2.0.0
├─ signedsource@1.0.0
├─ string-width@4.2.3
├─ supports-color@7.2.0
├─ to-fast-properties@2.0.0
├─ which-module@2.0.0
├─ wrap-ansi@6.2.0
├─ y18n@4.0.3
├─ yaml@1.10.2
├─ yargs-parser@18.1.3
└─ yargs@15.4.1
Done in 5.79s.

Wouldn't it be consistent und userfriendly to have the commands in the docs install the required version of graphql?

alunyov commented 3 years ago

But it still warns you, right?

I don't think documentation is the place where we should define/fix version of dependencies. package.json is the place for this. The most correct fix, is to publish 12.1.0 of the compiler/babel-plugin that works with v16 of GraphQL.

skleeschulte commented 3 years ago

My understanding is, that the documentation and the requirements defined in package.json for a specific version of a package should match. So adding the graphql version as it is defined as peer dependency in package.json to the commands in the documentation just fixes a shortcoming of the documentation. Currently, while package.json documents the requirement for graphql@^15.0.0, the documentation instructs to install the latest version.

alunyov commented 3 years ago

Thank you @skleeschulte and @AndrewIngram!

Sorry, for this back-and-forth. Again, I think the correct fix should be in the compiler, not in the docs. But it looks like the correct way may take longer.

We will update the docs, for now, it makes sense, thank you for PRs!

Streeterxs commented 3 years ago

So, no one found the root cause? Is closing this issue for a doc fix the right path?