Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(next-urql) - Node crashes with { ssr: true } #2307

Closed
longfellowone opened this issue Feb 25, 2022 · 5 comments · Fixed by #2308
Closed

(next-urql) - Node crashes with { ssr: true } #2307

longfellowone opened this issue Feb 25, 2022 · 5 comments · Fixed by #2308
Labels
bug 🐛 Oh no! A bug or unintented behaviour.

Comments

@longfellowone
Copy link

longfellowone commented Feb 25, 2022

urql version & exchanges:

  "dependencies": {
    "@urql/exchange-graphcache": "^4.3.6",
    "@urql/exchange-refocus": "^0.2.5",
    "graphql": "^15.8.0",
    "next": "^12.1.0",
    "next-urql": "^3.3.2",
    "react": "17.0.2",
    "react-dom": "17.0.2",
    "react-is": "^17.0.2",
    "urql": "^2.1.3"
  },

Steps to reproduce

  1. git clone https://github.com/longfellowone/tools.git
  2. cd tools/web
  3. git checkout bug
  4. npm install
  5. npm run dev
  6. Navigate to http://localhost:3000/
  7. Page should work load ok
  8. In tools/web/components/EmployeeOption.tsx uncomment line 18 // const [{ data }] = useEmployeeListQuery()
  9. Page does not load / Node crashes

Might have something to do with useEmployeeListQuery() needing to be deduped as its called within a loop

Expected behavior

Should work

Actual behavior

Page does not load / Crash

@longfellowone longfellowone added the bug 🐛 Oh no! A bug or unintented behaviour. label Feb 25, 2022
@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Feb 25, 2022

We often ask for a minimal reproducible example as we can't really go through the full code of everyone 😅 hence I made a small repro already here and will try to shed some light on the difference betweeen ssr: true and not.

As you might have noticed the new next method getXProps are happening outside of React land which makes us do a lot of passing stuff through to each other, this was not always the case. Before this getInitialProps was the way to go, this method had access to the React tree and we can invoke react-ssr-prepass which allows us to resolve all the useQuery calls. So this option is basically a more automated way but does not protect the user of making unintentional waterfalls

EDIT: I tried your repo and it just works for me, might be your node version
image

$ node -v
v16.7.0

@longfellowone
Copy link
Author

longfellowone commented Feb 25, 2022

my node version is 16.14.0

I just tried with 16.7.0 and am getting the same results. Did you do the step below?

  1. In tools/web/components/EmployeeOption.tsx uncomment line 18 // const [{ data }] = useEmployeeListQuery()

useEmployeeListQuery() is generated from graphql-code-generator if you use const [{ data }] = useQuery({ query: EmployeeListDocument }) in its place I still get the same result. (node hangs, and page does not load)

I also tried deploying to vercel and the page will not load. the only difference is commenting/uncommenting const [{ data }] = useQuery({ query: EmployeeListDocument }). I tried to slim down what I could to reproduce the issue

@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Feb 25, 2022

Well I think it is because as alluded to you are causing a 38 request waterfall and afaik next api functions (ssr) times out in 15 seconds 😅

EDIT: hmm looking into it as essentially these should be identical queries... hard to dive into the project as there's some redundant logic too 😅 this seems to be caused by the new use-sync-external-store this works on the pre 2.1.0 version of urql

@longfellowone
Copy link
Author

longfellowone commented Feb 25, 2022

Well I think it is because as alluded to you are causing a 38 request waterfall and afaik next api functions (ssr) times out in 15 seconds 😅

EDIT: hmm looking into it as essentially these should be identical queries... hard to dive into the project as there's some redundant logic too 😅 this seems to be caused by the new use-sync-external-store

Correct, they are all duplicate queries, so should it not be deduped? or does the dedup exchange have no effect with ssr?

redundant logic? sorry I am still new to this 😅 I'm open to suggestions

@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Feb 25, 2022

My best guess is currently that in prepass for concurrently running queries this line https://github.com/FormidableLabs/urql/blob/main/packages/react-urql/src/hooks/useQuery.ts#L84 should move into the scope of getSnapshot

EDIT: yup that's it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Oh no! A bug or unintented behaviour.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants