Undefined network status causes runtime errors

[lucas-teks]

Hello,

I just encountered a runtime error with the graphql-ppx use hook, it appears that in a certain case when the ~skipargument is passed as true the NetworkStatus gets undefined.

Here's how I fixed it, I suggest these changes:

src/@apollo/client/core/ApolloClient__Core_NetworkStatus.re

module NetworkStatus = {
module Js_ = {
-    type t = int;
+    type t = option(int);
};
[@bs.deriving jsConverter]

| [@bs.as 7] Ready
| [@bs.as 8] Error;

• let toJs: t => Js_.t = tToJs;

• let toJs: t => Js_.t = (t: t) => Some(tToJs(t));

• let fromJs: Js_.t => t = string => tFromJs(string)->Belt.Option.getExn;

• let fromJs: Js_.t => t =

• optionalString => {

• switch (optionalString) {

• | Some(string) => tFromJs(string)->Belt.Option.getExn

• | None => Ready

• };

• }; };

src/@apollo/client/core/ApolloClient__Core_Types.re

data,
error,
loading: js.loading,
-   networkStatus: js.networkStatus->NetworkStatus.fromJs,
+   networkStatus: Some(js.networkStatus)->NetworkStatus.fromJs,
};
};

Thanks for the great contribution!

[jeddeloh]

Awesome, thanks for finding this! I want to dig into this a little further and see exactly where issue is. Did I improperly type network status and it is supposed to be option(int)? Or did I improperly type something else and that type should have option(NetworkStatus.t)? Or is there no issue on our end and it's a bug in Apollo? I'll see if I can look into it later tonight.

[jeddeloh]

So it looks like it's indeed a bug (or bad type) in apollo client. So the question becomes, do we default to Ready like you have above, come up with a new variant like SkippedOrNotPresent, or just switch NetworkStatus.t to be optional everywhere? I lean toward one of the first two, but will sleep on it and see if we get any input in the meantime.

[jeddeloh]

And of those two, I'm kinda leaning toward SkippedOrNotPresent because it's the only one that actually conveys what's happening and doesn't require you to unwrap unnecessary optionals