feat: schema provider and operation session context using context/useReducer by acao · Pull Request #1359 · graphql/graphiql

acao · 2020-02-21T22:00:40Z

please take this and run with it tomorrow however you see fit!
it introduces:

useReducers which is meant to expose interfaces (to plugins eventually down the line, but for now we will supply them as props via the provider). glad to field suggestions on how to re-architect this!
a few useReducer related utilities inspired from previous redux workflows
a test utility for RTL, for testing provider output more readily
SchemaProvider, SchemaContext, etc. feel free to combine this provider into a migration provider like you were discussing earlier @zephraph

typescript types are not happy yet because I'm still wrapping my head around typescript + redux style patterns

note:

some features are temporarily disabled by this effort in the interest of further iterations before we release another alpha:

subscriptions support
multiple operations per query
persisted operations (though your previous query history works in the left sidebar, soon to be deprecated)

acao · 2020-02-28T19:59:04Z

@zephraph going to break some of these interfaces, since this PR breaks GraphiQL.state we might as well get rid of the GraphiQL.Logo pattern as well. See: pinned issue

acao · 2020-03-10T14:11:28Z

This is mostly ready, aside from some some cleanup.

Last step, was thinking of bringing schema context into DocExplorer using just render props, since it doesn't seem to be re-rendering from the top via GraphiQL.context.schema since we got rid of GraphiQL.state.schema

* Initial migration context wrapper * Pull out other static properties * Fix refs bug by properly accessing current

Co-Authored-By: Justin Bennett <[email protected]>

acao · 2020-03-17T00:27:14Z

@AlecAivazis welcome back! :) did you add yourself as a reviewer?

codecov · 2020-03-20T23:16:21Z

Codecov Report

Merging #1359 into master will decrease coverage by 22.04%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master    #1359       +/-   ##
===========================================
- Coverage   58.96%   36.92%   -22.05%     
===========================================
  Files          60       64        +4     
  Lines        3022     2941       -81     
  Branches      796      731       -65     
===========================================
- Hits         1782     1086      -696     
- Misses       1194     1833      +639     
+ Partials       46       22       -24

Impacted Files	Coverage Δ
...ages/graphiql/src/state/GraphiQLSchemaProvider.tsx	`0% <0%> (ø)`
packages/graphiql/src/components/QueryEditor.tsx	`0% <0%> (-63.74%)`	⬇️
packages/graphiql/src/components/GraphiQL.tsx	`0% <0%> (-58.9%)`	⬇️
packages/graphiql/src/components/ResultViewer.tsx	`0% <0%> (-73.81%)`	⬇️
packages/graphiql/src/state/common.ts	`0% <0%> (ø)`
packages/graphiql/src/components/ExecuteButton.tsx	`0% <0%> (-42.86%)`	⬇️
...ges/graphiql/src/state/GraphiQLSessionProvider.tsx	`0% <0%> (ø)`
...ackages/graphiql/src/components/VariableEditor.tsx	`0% <0%> (-62.83%)`	⬇️
packages/graphiql/src/state/useReducers.ts	`0% <0%> (ø)`
packages/graphiql/src/utility/CodeMirrorSizer.ts	`0% <0%> (-100%)`	⬇️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd77a51...c9d29e9. Read the comment docs.

sgrove · 2020-03-22T21:59:19Z

+  ? 'http://localhost:8080/graphql'
+  : 'https://swapi-graphql.netlify.com/.netlify/functions/index';
+
+export const initialReducerState: SchemaState = {


Is it possible to use discriminated unions so invalid states are impossible to represent?

A simplified example (Reason syntax since it's terser, but I think TS can represent this):

type schemaState = | Loading | Error(string) | Loaded(schema)

this is just temporary for the demo, but yes it is, via typescript. that would come in a later PR

lets talk about state in the plugin spec

cshaver

I haven't looked at this too closely yet, but I noticed one small thing where fetch is used:

cshaver · 2020-03-26T23:00:34Z

+      query: introspectionQuery,
+      operationName: introspectionQueryName,
+    }),
+    headers: { 'Content-Type': 'application/json', credentials: 'omit' },


Suggested change

headers: { 'Content-Type': 'application/json', credentials: 'omit' },

headers: { 'Content-Type': 'application/json' },

credentials: 'omit',

oops good catch!

cshaver · 2020-03-26T23:00:50Z

+    const rawResult = await fetch(schemaConfig.uri, {
+      method: 'post',
+      body: JSON.stringify(graphqlParams),
+      headers: { 'Content-Type': 'application/json', credentials: 'omit' },


Suggested change

headers: { 'Content-Type': 'application/json', credentials: 'omit' },

headers: { 'Content-Type': 'application/json' },

credentials: 'omit',

acao · 2020-04-05T00:50:01Z

@cshaver if you notice anything funky with tests, its because they're disabled temporarily for the RFC puproses

@ryan-m-walker is also around to help, he helped me start this PR!

ryan-m-walker · 2020-04-05T01:00:42Z

-        {options}
-      </div>
+export function ExecuteButton(props: ExecuteButtonProps) {
+  const [optionsOpen, setOptionsOpen] = useState<boolean>(false);


the generic isn't needed here. the type can be inferred

ryan-m-walker · 2020-04-05T01:20:52Z

+      payload: { value: variablesText, sessionId },
+    });
+  const executeOperation = async (
+    sessionState: SessionState,


it feels kind of strange that you have to pass the session state back into this function. Shouldn't the outer scope provide everything this function needs or are you trying to just encapsulate it all? lines like this one just feel strange to me:

https://github.com/graphql/graphiql/pull/1359/files#diff-de4c1a6e083a392d7c4acd996d95106dR49

ryan-m-walker · 2020-04-05T01:39:52Z

+    null,
+  );
+  const session = useSessionContext();
+  const operations = props.operations ?? [];


should we be getting operations from the session state now instead of props?

… an enum and discriminated union

* made errors consitent across providers * allowed for multiple errors * preserved error objects to allow more custom usage

* eslint-plugin-react-hooks * Move query facts out of session state * Rename sessionReducer

* used the return type of the action creators for the action types

acao · 2020-04-06T03:15:23Z

closing for #1468, in favor of using a fork for access management purposes.

acao requested a review from AlecAivazis February 21, 2020 22:08

acao added the graphiql label Feb 21, 2020

just-be-dev reviewed Mar 7, 2020

View reviewed changes

Comment thread packages/graphiql/src/state/MigrationContext.tsx Outdated

just-be-dev reviewed Mar 7, 2020

View reviewed changes

Comment thread packages/graphiql/src/state/MigrationContext.tsx Outdated

just-be-dev reviewed Mar 7, 2020

View reviewed changes

Comment thread packages/graphiql/src/state/MigrationContext.tsx Outdated

acao force-pushed the feat/use-context-hooks branch 2 times, most recently from 6b3e9af to 20ed39a Compare March 12, 2020 20:01

acao mentioned this pull request Mar 13, 2020

GraphiQL Plugins (2.0.0-beta.0) Meta Issue #983

Open

9 tasks

acao added this to the GraphiQL-1.0.0-beta milestone Mar 13, 2020

acao changed the title ~~feat: schema provider using context/useReducer~~ feat: schema provider and oepration session context using context/useReducer Mar 15, 2020

acao and others added 6 commits March 15, 2020 14:56

feat: schema provider using context/useReducer

1968a68

chore: cleanup, ts improvements

9452b12

WIP: feat: Add migration context to enable GraphiQL refactoring (#1380)

a8d35ca

* Initial migration context wrapper * Pull out other static properties * Fix refs bug by properly accessing current

feat: use context for schema and sesion state

91b4bf2

feat: results viewer as FC, purely context for op + results

d62507d

feat: doc explorer and query history using context

6ea0289

just-be-dev reviewed Mar 16, 2020

View reviewed changes

Comment thread packages/graphiql/src/components/ExecuteButton.tsx Outdated

chore: temporarily disable graphiql unit tests

a174c58

acao force-pushed the feat/use-context-hooks branch from 336c32c to a174c58 Compare March 16, 2020 15:41

chore: temporarily disable e2e tests for build

9f5c458

just-be-dev reviewed Mar 16, 2020

View reviewed changes

Comment thread packages/graphiql/src/components/QueryEditor.tsx Outdated

just-be-dev reviewed Mar 16, 2020

View reviewed changes

Comment thread packages/graphiql/src/components/QueryEditor.tsx Outdated

just-be-dev reviewed Mar 16, 2020

View reviewed changes

Comment thread packages/graphiql/src/components/QueryEditor.tsx

acao and others added 2 commits March 16, 2020 14:57

chore: use nullish coalescing operator, thanks @zephgraph

9d8ce9c

Co-Authored-By: Justin Bennett <[email protected]>

chore: cleanup ExecuteButton

aa832b1

acao changed the title ~~feat: schema provider and oepration session context using context/useReducer~~ feat: schema provider and operation session context using context/useReducer Mar 17, 2020

feat: use static local/remote schemas

246c51e

acao force-pushed the feat/use-context-hooks branch from 31fc6e0 to 246c51e Compare March 17, 2020 15:58

Merge branch 'master' into feat/use-context-hooks

97f347f

sgrove reviewed Mar 22, 2020

View reviewed changes

cshaver reviewed Mar 27, 2020

View reviewed changes

acao added rfc Request for Comment work-in-progress not ready to merge yet, PR is available for demonstration/review purposes labels Apr 1, 2020

acao mentioned this pull request Apr 1, 2020

GraphiQL 2.0.0 beta Roadmap (monaco, redesign and plugin API!) #1446

Closed

23 tasks

acao added the please review! label Apr 4, 2020

ryan-m-walker reviewed Apr 5, 2020

View reviewed changes

ryan-m-walker and others added 8 commits April 5, 2020 22:36

feat: updated session reducer to have better typescript typings using…

dfc4fbd

… an enum and discriminated union

feat: updating typings and structure of schema reducer

99c7f4c

fix: removed comment

ac9dc70

feat: added observable support back to fetcher function

754c327

feat: standardized error handling

1e6f5d3

* made errors consitent across providers * allowed for multiple errors * preserved error objects to allow more custom usage

feat: schema provider - query facts out of session state, unwrap reducer

ea276e8

* eslint-plugin-react-hooks * Move query facts out of session state * Rename sessionReducer

fix: fixed import pathing issue

4b3ef26

feat: action types

c9d29e9

* used the return type of the action creators for the action types

acao closed this Apr 6, 2020

acao reopened this Apr 6, 2020

acao closed this Apr 6, 2020

	headers: { 'Content-Type': 'application/json', credentials: 'omit' },
	headers: { 'Content-Type': 'application/json' },
	credentials: 'omit',

Conversation

acao commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acao commented Feb 28, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acao commented Mar 10, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acao commented Mar 17, 2020

Uh oh!

codecov Bot commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cshaver left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acao commented Apr 5, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryan-m-walker Apr 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acao commented Apr 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

acao commented Feb 21, 2020 •

edited

Loading

codecov Bot commented Mar 20, 2020 •

edited

Loading

ryan-m-walker Apr 5, 2020 •

edited

Loading

acao commented Apr 6, 2020 •

edited

Loading