jeddeloh / rescript-apollo-client

ReScript bindings for the Apollo Client ecosystem
MIT License
127 stars 18 forks source link

Updating to Latest Dependencies Rescript version 11, Rescript/React version 12 #156

Open GTDev87 opened 2 months ago

GTDev87 commented 2 months ago

Updated to latest versions

@apollo/client: 3.5.0 -> 3.10.8 @reasonml-community/graphql-ppx: 1.0.0 -> 1.2.3 @rescript/react: 0.11.0 -> 0.12.2 rescript: 10.1.2 -> 11.1.2 graphql: 14.0.0 -> 16.9.0 react: 18.2.0 -> 18.3.1 react-dom: 18.2.0 -> 18.3.1

Breaking change was new rescript uncurried mode, should be backward compatible.

GTDev87 commented 2 months ago

I wonder if uncurried: false is required if it is added as a dependency in a project.

jeddeloh commented 2 months ago

Sorry, totally missed notifications on this. This is really all we need? That would be great. That was my initial hope, but haven't looked into it in a while. Have you tried running the examples directory with this?

zatchheems commented 3 weeks ago

hello! just chiming in to say that I have a project that relies heavily on this library and this is the last dependency preventing us from upgrading to Rescript 11.

as of right now there's just one error related to currying I can't seem to get around even with changes made in this PR:

FAILED: src/ApolloClient__Utils.cmj

  We've found a bug for you!
  /<project directory>/client/node_modules/rescript-apollo-client/src/ApolloClient__Utils.res:11:72-15:3

   9 │ external asJson: 'any => Js.Json.t = "%identity"
  10 │
  11 │ let safeParse: ('jsData => 'data) => Types.safeParse<'data, 'jsData> =
     │ (parse, jsData) =>
  12 │   switch parse(jsData) {
  13 │   | data => Ok(data)
  14 │   | exception Js.Exn.Error(error) => Error({value: jsData->asJson, erro
     │ r: error})
  15 │   }
  16 │
  17 │ let safeParseAndLiftToCommonResultProps: (

  This function expected 1 argument, but got 2

@jeddeloh I know you don't really maintain this library anymore, any thoughts on maintenance or drop-in alternatives? just wondering what the future of this library is/trying to avoid doing too much rework on my project 🙂

jeddeloh commented 3 weeks ago

@zatchheems Yeah, I'm not really maintaining this anymore, but I'm very happy to help anyone that would like to take a stab at this. With the error you're getting, how sure are you that this isn't just the first error into a cascade of errors until the library has been converted to uncurried? If you're getting an error at all, I assume it means that just simply setting uncurried: false in the bsconfig of the library is not sufficient. @GTDev87 are you having success running this?

@zatchheems have you tried just going through and converting stuff to uncurried? I really doubt it would take all that long.

zatchheems commented 3 weeks ago

how sure are you that this isn't just the first error into a cascade of errors until the library has been converted to uncurried?

not sure at all! I tested it very briefly so it's likely just the first thread I'll need to pull.

I took a look at https://github.com/jeddeloh/rescript-apollo-client/pull/153 which seems to have addressed much of converting stuff to uncurried (Edit: or at least til that branch was force-pushed and those commits disappeared). I can take a crack at it.

jeddeloh commented 2 weeks ago

That is unfortunate. If I remember correctly, it was indeed basically done. @AlexMoutonNoble do you know what happened there?

AlexMouton commented 2 weeks ago

That is unfortunate. If I remember correctly, it was indeed basically done. @AlexMoutonNoble do you know what happened there?

Hi Joel Ahh. I am not very confident in my approach in that branch. I would approach with caution. Belt is discouraged in 10+, and some of the flattening of functions t => x to t, x => may not be useful either. As I remember it, the ppx generation was also troubled so having this library compile didnt get you very close.

Happy to help too as I can, but will be largely out of contact for the next week or more.

zatchheems commented 2 weeks ago

just to keep this conversation moving, I have been slowly making my way through the necessary updates. I'll create my own PR when that's ready.

one q: I've copied the bsconfig.json to rescript.json and removed the uncurried: false flag from both. this silences the Rescript 11 compiler warning and should theoretically be backwards-compatible. is this a good solution or too much of a kludge just to avoid releasing a new major version?

jeddeloh commented 2 weeks ago

Awesome! I think we can forget about backwards compatibility at this point and just shoot for a new major version. There's nothing no new features coming in the near future so people can always just stay on an older version if necessary. Also, sorry for the slow reply and heads up that I'm going in for surgery tomorrow so may be a bit slow to respond this week.

zatchheems commented 1 week ago

@jeddeloh one last hurdle before I open a new PR: I've been trying to make sense of uncurrying calls to onClearStore in ApolloClient__Core_ApolloClient.res and am struggling with an error.

earlier in the file, I uncurried the call like so:

  let onClearStore = t => (~cb) => jsClient->Js_.onClearStore(t, ~cb)

a record is passed as the second argument to preserveJsPropsAndContext and I'm not sure what to do here:

  //<user path>/src/@apollo/client/core/ApolloClient__Core_ApolloClient.res:1064:7-18

  1062 ┆ extract,
  1063 ┆ mutate,
  1064 ┆ onClearStore, <--
  1065 ┆ onResetStore,
  1066 ┆ query,

  This function expected 2 arguments, but got 1

this will likely be the same issue for onResetStore as it has a similar function signature. help appreciated!