quantified-uncertainty / metaforecast

Fetch forecasts from prediction markets/forecasting platforms to make them searchable. Integrate these forecasts into other services.
https://metaforecast.org/
MIT License
56 stars 5 forks source link

GraphQL #60

Closed berekuk closed 2 years ago

berekuk commented 2 years ago

Pushing this just to show the progress so far. Not ready to merge yet.

Related issues: #32, #59

Notes:

  1. Pothos is good! Some minor issues with docs, but nothing critical; Prisma plugin and Relay plugin work fine, which is great.
  2. urql with SSR is... ugh. Dealing with it right now (haven't pushed the related commit yet). I had to read through the next-urql source code to figure it out.
    • next-urql uses the deprecated getInitialProps from older Next.js as a default mechanism but has the solution for more modern getStaticProps or getServerSideProps
    • we'll have to list all requests for GraphQL data in getServerSideProps manually (e.g., deeply nested components which need some portion of data won't magically SSR with all their useQuery data by themselves)
      • this is not urql's fault, since the alternative is to render everything on the server twice, which is slow
    • I don't like the amount of code which will be necessary for every getServerSideProps function (urqlState: ssrCache.extractData(), initUrqlClient, etc.); I'll try to come up with some helper functions to make the DX smoother
  3. I'm replacing searchAccordingToQueryData with a GraphQL query, to abstract away all search logic from the frontend code. This requires substantial refactorings, hope to push the working implementation in a day or two.
vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
metaforecast ✅ Ready (Inspect) Visit Preview Apr 21, 2022 at 8:30PM (UTC)
netlify[bot] commented 2 years ago

Deploy Preview for metaforecast failed.

Name Link
Latest commit 3ef786db4bdd1e456c6c890e2fe766ff7752efe6
Latest deploy log https://app.netlify.com/sites/metaforecast/deploys/6261be86cb55ef00089a48e4
berekuk commented 2 years ago

80% complete, 80% to go.

Needs fixing:

Also:

berekuk commented 2 years ago

urql-custom-scalars-exchange

It works, but it's incompatible with SSR, since Next.js can't serialize Date objects.

There's a workaround with superjson, but I don't think that an extra dependency is worth it.

I left urql-custom-scalars-exchange config & schema introspection code for the future since I already wrote it when I realized this, and schema introspection might be needed for better urql caching anyways.

For now I'm just serializing timestamp as an int (which is better/safer than a date string) and decode it with new Date(ts * 1000) on the frontend.

creating dashboards

Done.


Pushed the new changes, https://metaforecast-git-graphql-quantified-uncertainty.vercel.app/ is mostly functional now. (SSR on the frontpage is currently broken, will investigate).

berekuk commented 2 years ago

Ok, I think this is done!

Docs are in docs/graphql.md, and I added a description for the most of graphql fields (sometimes copying from specification.json, which can be deprecated soon, since GraphQL is better for this kind of documentation).


Notes on SSR.

I still don't like how SSR is implemented, especially making a choice of "do I pass the page data from getServerSideProps in urqlState cache or in page props".

Passing in props means that the page is not really dynamic (I consider initialBlah props with useState to be an anti-pattern which should be avoided if possible). Which is mostly fine, except for the frontpage, which is dynamic. It also means that we won't be able to benefit from urql's caching abilities in the future, and caching is a big selling point for graphql; though it's more useful for SPA-like rich UI features, which we don't have yet.

On the other hand, passing in urqlState means that we have to do the query in getServerSideProps and then do the same query with the useQuery on client. Which means that if the document or variables for one of these queries ever changes on the future and we forget to change the other call then SSR would break (i.e. degrade to client-side query, the page won't break entirely, but it would break silently and we might not notice it for a while).

Issues with both of these approaches are non-critical, and it's not hard to change the code from using one approach to another. But I don't like having this choice, it feels like extra cognitive load for no clear benefit.

I don't have any good ideas for how to deal with it yet, and maybe I'll just get adjusted to it over time.


Notes on types.

NunoSempere commented 2 years ago

Any idea why the netlify build preview is failing? The vercel one is working: https://metaforecast-lfn840cvn-quantified-uncertainty.vercel.app

NunoSempere commented 2 years ago

Ok, I think this is done!

Should I merge, or any last checks?

NunoSempere commented 2 years ago

Your notes on SSR (server-side rendering, I'm guessing) went a bit over my head. Any actionables, besides writing this down somewhere so that we remember this when we come back to it in a few months?

berekuk commented 2 years ago

Any idea why the netlify build preview is failing?

Probably because the build now requires prisma generate, which is run in npm run build (see scripts in package.json) but netlify runs next build instead.

Anyway, I disabled netlify builds yesterday since we don't use it anymore.

Your notes on SSR (server-side rendering, I'm guessing) went a bit over my head.

That's fine. It's kinda hard to explain clearly without repeating all graphql/urql docs, and I think I'm not doing anything non-standard here.

Any actionables, besides writing this down somewhere so that we remember this when we come back to it in a few months?

No, my notes above are "writing this down somewhere", I should've been more clear about that. No actions needed. (I slightly dislike putting stuff like this in docs/ in the repo, since code usually changes faster than documentation and documentation can get out of sync with reality or with our understanding of best practices.)