From 17d23387656e2f6aaa0047647b456a2c632a9670 Mon Sep 17 00:00:00 2001 From: Eduardo Lundgren Date: Thu, 24 Aug 2023 13:08:23 -0700 Subject: [PATCH 1/6] Highlight Issue: Todo example with Relay v15.0.0 and @required field - Updated the Todo demo to demonstrate with Relay v15.0.0 - Added the @required field which was experimental in v12.0.0 - Contrasted with the official Relay repo's Todo demo to show that @required is not functioning as expected in v15.0.0. Exceptions should have been thrown, but aren't (see attached screenshot for a comparison). - This PR is to bring attention to this observed issue with the @required field at relay-hooks with Relay v15.0.0. --- .../relay-hook-example/todo/data/schema/nodes.js | 2 +- examples/relay-hook-example/todo/js/app.js | 9 ++++++++- .../relay-hook-example/todo/js/components/Todo.js | 2 +- examples/relay-hook-example/todo/package.json | 12 +++++++++--- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/examples/relay-hook-example/todo/data/schema/nodes.js b/examples/relay-hook-example/todo/data/schema/nodes.js index 5e5fad7f..2ec1d07e 100644 --- a/examples/relay-hook-example/todo/data/schema/nodes.js +++ b/examples/relay-hook-example/todo/data/schema/nodes.js @@ -65,7 +65,7 @@ const GraphQLTodo = new GraphQLObjectType({ id: globalIdField('Todo'), text: { type: new GraphQLNonNull(GraphQLString), - resolve: (todo: Todo): string => todo.text, + resolve: (todo: Todo): string => null, }, complete: { type: new GraphQLNonNull(GraphQLBoolean), diff --git a/examples/relay-hook-example/todo/js/app.js b/examples/relay-hook-example/todo/js/app.js index 3496c4c5..ca342d8c 100644 --- a/examples/relay-hook-example/todo/js/app.js +++ b/examples/relay-hook-example/todo/js/app.js @@ -83,10 +83,17 @@ function fetchQuery( sink.next(data); sink.complete(); }); - }) + }); } +const requiredFieldLogger = (fieldPath, fieldName) => { + console.error( + `Required field "${fieldName}" is null at path "${fieldPath}".`, + ); +}; + const modernEnvironment: Environment = new Environment({ + requiredFieldLogger, network: Network.create(fetchQuery), store: new Store(new RecordSource()), }); diff --git a/examples/relay-hook-example/todo/js/components/Todo.js b/examples/relay-hook-example/todo/js/components/Todo.js index 3277b663..201ce393 100644 --- a/examples/relay-hook-example/todo/js/components/Todo.js +++ b/examples/relay-hook-example/todo/js/components/Todo.js @@ -39,7 +39,7 @@ const fragmentSpecTodo = graphql` fragment Todo_todo on Todo { complete id - text + text @required(action: THROW) } `; const fragmentSpecUser = graphql` diff --git a/examples/relay-hook-example/todo/package.json b/examples/relay-hook-example/todo/package.json index d1d400ff..f5c33598 100644 --- a/examples/relay-hook-example/todo/package.json +++ b/examples/relay-hook-example/todo/package.json @@ -2,7 +2,7 @@ "private": true, "scripts": { "start": "babel-node ./server.js", - "build": "relay-compiler --src ./js/ --schema ./data/schema.graphql --artifactDirectory ./__generated__/relay", + "build": "relay-compiler", "flow-typed": "flow-typed install --flowVersion=0.94.0", "update-schema": "babel-node ./scripts/updateSchema.js", "lint": "eslint ./ --cache" @@ -16,7 +16,7 @@ "prop-types": "^15.7.2", "react": "^16.12.0", "react-dom": "^16.8.4", - "relay-runtime": "12.0.0", + "relay-runtime": "15.0.0", "relay-hooks": "6.0.0", "whatwg-fetch": "3.0.0", "isomorphic-fetch": "2.2.1", @@ -47,7 +47,7 @@ "flow-bin": "^0.94.0", "flow-typed": "^2.5.1", "prettier": "^1.16.4", - "relay-compiler": "12.0.0", + "relay-compiler": "15.0.0", "webpack": "^5.56.1", "webpack-dev-server": "^3.2.1" }, @@ -56,5 +56,11 @@ "trailingComma": "all", "bracketSpacing": false, "jsxBracketSameLine": true + }, + "relay": { + "src": "./js/", + "schema": "./data/schema.graphql", + "artifactDirectory": "./__generated__/relay", + "language": "javascript" } } From e9f024ad23fbd5330d6bb0b2c5ab468c0c43e2e0 Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 25 Aug 2023 17:05:02 +0200 Subject: [PATCH 2/6] transformed the non-mandatory text field on the server side Having the required field, the server change that returns null as a value, causes an error in the network response --- .../todo/data/schema.graphql | 82 ++++++++++++++----- .../todo/data/schema/nodes.js | 2 +- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/examples/relay-hook-example/todo/data/schema.graphql b/examples/relay-hook-example/todo/data/schema.graphql index c3306a18..4ad5a99e 100644 --- a/examples/relay-hook-example/todo/data/schema.graphql +++ b/examples/relay-hook-example/todo/data/schema.graphql @@ -39,38 +39,58 @@ type Mutation { addTodo(input: AddTodoInput!): AddTodoPayload changeTodoStatus(input: ChangeTodoStatusInput!): ChangeTodoStatusPayload markAllTodos(input: MarkAllTodosInput!): MarkAllTodosPayload - removeCompletedTodos(input: RemoveCompletedTodosInput!): RemoveCompletedTodosPayload + removeCompletedTodos( + input: RemoveCompletedTodosInput! + ): RemoveCompletedTodosPayload removeTodo(input: RemoveTodoInput!): RemoveTodoPayload renameTodo(input: RenameTodoInput!): RenameTodoPayload } -"""An object with an ID""" +""" +An object with an ID +""" interface Node { - """The id of the object.""" + """ + The id of the object. + """ id: ID! } -"""Information about pagination in a connection.""" +""" +Information about pagination in a connection. +""" type PageInfo { - """When paginating forwards, are there more items?""" + """ + When paginating forwards, are there more items? + """ hasNextPage: Boolean! - """When paginating backwards, are there more items?""" + """ + When paginating backwards, are there more items? + """ hasPreviousPage: Boolean! - """When paginating backwards, the cursor to continue.""" + """ + When paginating backwards, the cursor to continue. + """ startCursor: String - """When paginating forwards, the cursor to continue.""" + """ + When paginating forwards, the cursor to continue. + """ endCursor: String } type Query { user(id: String): User - """Fetches an object given its ID""" + """ + Fetches an object given its ID + """ node( - """The ID of an object""" + """ + The ID of an object + """ id: ID! ): Node } @@ -110,35 +130,57 @@ type RenameTodoPayload { } type Todo implements Node { - """The ID of an object""" + """ + The ID of an object + """ id: ID! - text: String! + text: String complete: Boolean! } -"""A connection to a list of items.""" +""" +A connection to a list of items. +""" type TodoConnection { - """Information to aid in pagination.""" + """ + Information to aid in pagination. + """ pageInfo: PageInfo! - """A list of edges.""" + """ + A list of edges. + """ edges: [TodoEdge] } -"""An edge in a connection.""" +""" +An edge in a connection. +""" type TodoEdge { - """The item at the end of the edge""" + """ + The item at the end of the edge + """ node: Todo - """A cursor for use in pagination""" + """ + A cursor for use in pagination + """ cursor: String! } type User implements Node { - """The ID of an object""" + """ + The ID of an object + """ id: ID! userId: String! - todos(status: String = "any", after: String, first: Int, before: String, last: Int): TodoConnection + todos( + status: String = "any" + after: String + first: Int + before: String + last: Int + ): TodoConnection totalCount: Int! completedCount: Int! } diff --git a/examples/relay-hook-example/todo/data/schema/nodes.js b/examples/relay-hook-example/todo/data/schema/nodes.js index 2ec1d07e..7a23cb79 100644 --- a/examples/relay-hook-example/todo/data/schema/nodes.js +++ b/examples/relay-hook-example/todo/data/schema/nodes.js @@ -64,7 +64,7 @@ const GraphQLTodo = new GraphQLObjectType({ fields: { id: globalIdField('Todo'), text: { - type: new GraphQLNonNull(GraphQLString), + type: GraphQLString, resolve: (todo: Todo): string => null, }, complete: { From 835a3faa898e66c21f35d33a3f7adb4f7b127d19 Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 25 Aug 2023 17:07:24 +0200 Subject: [PATCH 3/6] restored the use of the fragment that used the @required directive without this change the directive was ignored as it was not used --- examples/relay-hook-example/todo/js/components/Todo.js | 4 ++-- examples/relay-hook-example/todo/js/components/TodoList.js | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/examples/relay-hook-example/todo/js/components/Todo.js b/examples/relay-hook-example/todo/js/components/Todo.js index 201ce393..073179a3 100644 --- a/examples/relay-hook-example/todo/js/components/Todo.js +++ b/examples/relay-hook-example/todo/js/components/Todo.js @@ -54,8 +54,8 @@ const fragmentSpecUser = graphql` const Todo = props => { const user = useFragment(fragmentSpecUser, props.user); console.log('props.todo', props.todo); - //const todo = useFragment(fragmentSpecTodo, props.todo); - const {todo} = props; + const todo = useFragment(fragmentSpecTodo, props.todo); + //const {todo} = props; const [isEditing, setIsEditing] = useState(false); const [mutateChange] = useMutation(mutationChange); diff --git a/examples/relay-hook-example/todo/js/components/TodoList.js b/examples/relay-hook-example/todo/js/components/TodoList.js index b9d3692c..ac8c2575 100644 --- a/examples/relay-hook-example/todo/js/components/TodoList.js +++ b/examples/relay-hook-example/todo/js/components/TodoList.js @@ -51,9 +51,7 @@ const fragmentSpec = graphql` const fragmentSpecList = graphql` fragment TodoList_edges on TodoEdge @relay(plural: true) { node { - complete - id - text + ...Todo_todo } } `; From 44e7c17def252439b0b1f7b8bbf93d3ae4e1ad17 Mon Sep 17 00:00:00 2001 From: morrys Date: Fri, 25 Aug 2023 18:07:35 +0200 Subject: [PATCH 4/6] WIP solution In relay "snapshot.missingData" is checked while here in relay-hooks "snapshot.missingRequiredFields". To investigate the different behavior between concurrent and non-concurrent mode. --- src/FragmentResolver.ts | 23 +++++++++++++++++++++++ src/QueryFetcher.ts | 11 ++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/FragmentResolver.ts b/src/FragmentResolver.ts index 7718b884..8c483c75 100644 --- a/src/FragmentResolver.ts +++ b/src/FragmentResolver.ts @@ -21,6 +21,7 @@ import { getPaginationVariables, getRefetchMetadata, getValueAtPath, + handlePotentialSnapshotErrors, } from 'relay-runtime'; import { Fetcher, fetchResolver } from './FetchResolver'; import { getConnectionState, getStateFromConnection } from './getConnectionState'; @@ -338,6 +339,10 @@ export class FragmentResolver { // useFragment this.result = data; } + const snap = this.resolverData.snapshot; + if (snap) { + this._throwOrLogErrorsInSnapshot(snap); + } this._subscribeResolve && this._subscribeResolve(this.result); } @@ -621,4 +626,22 @@ export class FragmentResolver { this.refreshHooks(); return disposable; }; + + _throwOrLogErrorsInSnapshot(snapshot: any) { + if (Array.isArray(snapshot)) { + snapshot.forEach((s) => { + if (s.missingRequiredFields) { + handlePotentialSnapshotErrors(this._environment, s.missingRequiredFields, s.relayResolverErrors); + } + }); + } else { + if (snapshot.missingRequiredFields) { + handlePotentialSnapshotErrors( + this._environment, + snapshot.missingRequiredFields, + snapshot.relayResolverErrors, + ); + } + } + } } diff --git a/src/QueryFetcher.ts b/src/QueryFetcher.ts index d2f622ca..7339b017 100644 --- a/src/QueryFetcher.ts +++ b/src/QueryFetcher.ts @@ -8,6 +8,7 @@ import { OperationDescriptor, GraphQLTaggedNode, Variables, + handlePotentialSnapshotErrors, } from 'relay-runtime'; import { Fetcher, fetchResolver } from './FetchResolver'; import { FetchPolicy, RenderProps, QueryOptions, Options } from './RelayHooksTypes'; @@ -203,10 +204,18 @@ export class QueryFetcher resolveResult(): void { const { error, isLoading } = this.fetcher.getData(); + const snapshot: any = this.snapshot; + if (snapshot && snapshot.missingRequiredFields) { + handlePotentialSnapshotErrors( + this.environment, + snapshot.missingRequiredFields, + snapshot.relayResolverErrors, + ); + } this.result = { retry: this.retry, error, - data: this.snapshot ? this.snapshot.data : null, + data: snapshot ? snapshot.data : null, isLoading, }; } From cf5e4aafa62c8ca296141a489f14848deeee9747 Mon Sep 17 00:00:00 2001 From: morrys Date: Thu, 7 Sep 2023 11:57:05 +0200 Subject: [PATCH 5/6] update peerDependencies relay-runtime from ^13.0.2 to ^13.2.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fb30f141..2df2a728 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ }, "peerDependencies": { "react": "^16.9.0 || ^17 || ^18", - "relay-runtime": "^13.0.2 || ^14.0.0 || ^15.0.0" + "relay-runtime": "^13.2.0 || ^14.0.0 || ^15.0.0" }, "devDependencies": { "babel-preset-fbjs": "^3.3.0", From 0e64f9fbfee35f869f971b8292755d2b42e9de51 Mon Sep 17 00:00:00 2001 From: morrys Date: Thu, 7 Sep 2023 11:57:45 +0200 Subject: [PATCH 6/6] Update package-lock.json --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 44f620c1..0086b9a2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -65,7 +65,7 @@ }, "peerDependencies": { "react": "^16.9.0 || ^17 || ^18", - "relay-runtime": "^13.0.2 || ^14.0.0 || ^15.0.0" + "relay-runtime": "^13.2.0 || ^14.0.0 || ^15.0.0" } }, "node_modules/@babel/cli": {