reasonml-community / reason-apollo-hooks

Deprecated in favor of https://github.com/reasonml-community/graphql-ppx
https://reasonml-community.github.io/reason-apollo-hooks/
MIT License
137 stars 28 forks source link

useEffect is called on every re-render when using simple as a dependency #55

Closed Pet3ris closed 4 years ago

Pet3ris commented 4 years ago

Hi There,

I'm facing an issue very similar to this one.

However, my hook is related to the simple argument of the query, not the full argument. It does unfortunately trigger the hook on every re-render.

Any thoughts on why that is if the underlying query value is not changing?

Qziem commented 4 years ago

I think that better to use full in useEffect1 argrument. But you can still use simple inside. Try:

let (simple, full) = UserQuery.use();
React.useEffect1(
  () => {
    Js.log2("useEffect called", simple);
    None;
  },
  [|full|],
);

I tried this and looks like its called only when full is changed, not every rerender. I think its becouse full its record which is immutable structure.

Qziem commented 4 years ago

I probably know why is that: In Query.re calculation full is in React.useMemo which means that it is calculated only when its change. But calulate simple is not in React.useMemo - but in my opinion it should be (like in Mutation.re)

Pet3ris commented 4 years ago

@kamilkuz - that's AMAZING! 🙌 Thank you - love a simple fix. I will leave it open for now before the authors can verify if it's seen as a bug or not.

fakenickels commented 4 years ago

Hey there! Sorry about the delay we have been out for a conference in this weekend and will get back here soon.

I agree that simple should be also in a React.useMemo!