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

[relay-compiler 13] compiler panic for client-only operations: Expected to have a operation_text operation. #3760

Open artola opened 2 years ago

artola commented 2 years ago

relay-compiler is not able to discover the client schema extensions, same were working in v12. Is necessary some other config setting to enable them? or just enough to drop the file into the src folder as it was before?

[ERROR] ✖︎ The type `Query` has no field `local`.

  src/trial.js:9:5
    8 │   query trial2Query {
    9 │     local {
      │     ^^^^^
   10 │       color

[ERROR] Compilation failed.
[ERROR] Failed to build:
 - Validation errors:
 - The type `Query` has no field `local`.:src/trial.js:82:87

Demo repo linked here: https://github.com/artola/rc-issue-3760

artola commented 2 years ago

@sibelius @alunyov See repo linked above. This issue is relay-compiler specific, nothing else involved.

maraisr commented 2 years ago

You need to add relay config json file and point it to where it can find the schema.

lpalmes commented 2 years ago

@artola I had to add another field (schemaExtensions) in the config to indicate where extensions are hosted, in your case the src folder. You can find the options here

artola commented 2 years ago

@maraisr I have the config, but I was missing the new schemaExtensions that @lpalmes mention above. It it is not like this in the documentation like it was up to v12.

https://relay.dev/docs/guides/client-schema-extensions/#extending-the-server-schema

Extending the server schema#
To extend the server schema, create a new .graphql file inside your --src directory. Let's call it ./src/clientSchema.graphql.

@maraisr @lpalmes Thanks a lot!

artola commented 2 years ago

Adding the schemaExtensions to the config works, as it find it. But, the file in my example contains 2 queries, and written as it is, works in v12 but fails in v13.

querying files to compile...
[default] compiling...
thread 'main' panicked at 'Expected to have a operation_text operation for `trial2Query`', crates/relay-compiler/src/build_project/generate_artifacts.rs:278:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error Command failed with exit code 101.

See the file to be compiled:

import graphql from 'relay-runtime';

graphql`
  query trial1Query {
    hero {
      name
    }
  }

  query trial2Query {
    local {
      color
    }
  }
`;

@alunyov If I just query for the local data, it fails:

graphql`
  query trial2Query {
    local {
      color
    }
  }
`;

But if I keep some other fields, then works:

graphql`
  query trial1Query {
    hero {
      name
    }
    local {
      color
    }
  }
`;
artola commented 2 years ago

@alunyov This is a hack in our codebase reported by @sibelius (2 years ago), still nasty but valid workaround: Can be done better now? Apart from this "extra", the original issue is solved. I would appreciate some update on this topic, otherwise can be closed.

// SOLUTION: add `schemaExtensions` to the config.
// @see https://github.com/facebook/relay/issues/3760#issuecomment-1014907033
//
// SOLUTION: "empty" queries break, therefore workaround with @sibelius hack.
// @see https://github.com/facebook/relay/issues/3760#issuecomment-1014930121
graphql`
  query trial1Query {
    hero {
      name
    }
  }

  query trial2Query {
    # Needed to add __typename
    # See Relay issue here: https://github.com/facebook/relay/issues/2471#issuecomment-624238501
    ... on Query {
      __typename
    }

    local {
      color
    }
  }
`;
alunyov commented 2 years ago

Thanks @artola, I think we also had similar reports internally with these client-only operations. I've renamed the issue.

alunyov commented 2 years ago

This should be implemented here: 124435968e54

artola commented 2 years ago

This should be implemented here: 1244359

@alunyov It seems that the compiler in v14 can handle this kind of "local" only queries. For example, I can not see a persisted query for a local only query.

But, it fails later in runtime, we are trying to fetch queries that has not id neither text related to this local only query. Like if the runtime does not understand that it is client only and still calls the fetchFn.

This is an example when I re-add the hack used above:

    ... on Query {
      __typename
    }

We can see that without, the query was of type ClientRequest and has not id or text. The problem, if that the runtime still was calling the fetchFn on the NetworkLayer, and this leads to the server answering with 400 error as it is not passing the validation rules (we use text in development and id in production).

@@ -1,5 +1,5 @@
 /**
- * @generated SignedSource<<5e7fed0da95afdcca36e761f25a50914>>
+ * @generated SignedSource<<befd76c8ecf8bf27b1a02d077408187b>>
  * @lightSyntaxTransform
  * @nogrep
  */
@@ -8,10 +8,11 @@
 /* eslint-disable */
 // @ts-nocheck

-import { ClientRequest, ClientQuery } from 'relay-runtime';
+import { ConcreteRequest, Query } from 'relay-runtime';
 export type RegisteredState = "COMPLETED" | "PENDING" | "UNKNOWN";
 export type useBusinessLightQuery$variables = {};
 export type useBusinessLightQuery$data = {
+  readonly __typename: string;
   readonly local: {
     readonly registered: RegisteredState;
   } | null;
@@ -21,8 +22,15 @@ export type useBusinessLightQuery = {
   variables: useBusinessLightQuery$variables;
 };

-const node: ClientRequest = (function(){
+const node: ConcreteRequest = (function(){
 var v0 = [
+  {
+    "alias": null,
+    "args": null,
+    "kind": "ScalarField",
+    "name": "__typename",
+    "storageKey": null
+  },
   {
     "kind": "ClientExtension",
     "selections": [
@@ -65,16 +73,16 @@ return {
     "selections": (v0/*: any*/)
   },
   "params": {
-    "cacheID": "bbf00a5ea0535b25abfff10d0637101d",
+    "cacheID": "cc350be8995bba2a4c821e133ce8b7fc",
     "id": null,
     "metadata": {},
     "name": "useBusinessLightQuery",
     "operationKind": "query",
-    "text": null
+    "text": "query useBusinessLightQuery {\n  __typename\n}\n"
   }
 };
 })();

-(node as any).hash = "ff04343fa65411bafa569dacc1990ffa";
+(node as any).hash = "7fbfe3b7adaa282724682a87a95e737b";

 export default node;
alunyov commented 2 years ago

@artola i think this is expected behavior. These client-only queries should not be sent and executed on the server. The server is not aware of the local client extensions. These queries should be resolved only on the client with the data provided with local updaters. We also using different flow-type for these queries where you can pass it only go 'useLazyLoadQuery'.

artola commented 2 years ago

@artola i think this is expected behavior. These client-only queries should not be sent and executed on the server. The server is not aware of the local client extensions. These queries should be resolved only on the client with the data provided with local updaters. We also using different flow-type for these queries where you can pass it only go 'useLazyLoadQuery'.

My expectation was that if the query is client only, the network layer should not call the fetchFn. But, it is not like this ... then, if I understand, we should pass fetchPolicy: "store-only" to avoid that call.

In our case using fetchQuery, it is not possible:

https://github.com/facebook/relay/blob/b193cad73eddc6daccaf986d769e895455e269a9/packages/relay-runtime/query/fetchQuery.js#L120

https://github.com/facebook/relay/blob/b193cad73eddc6daccaf986d769e895455e269a9/packages/relay-runtime/util/RelayRuntimeTypes.js#L73

alunyov commented 2 years ago

The idea here is to be intentional with these client only queries, so they should not be used with APIs that may send a network request. But now I also think we'll need to add a new hook - useLocalQuery (same as useLazyLoadQuey with store-only), and enforce statically that client queries only used with client hook.