search

UnlyEd / next-right-now

Flexible production-grade boilerplate with Next.js 11, Vercel and TypeScript. Includes multiple opt-in presets using Storybook, Airtable, GraphQL, Analytics, CSS-in-JS, Monitoring, End-to-end testing, Internationalization, CI/CD and SaaS B2B multi single-tenancy (monorepo) support
https://unlyed.github.io/next-right-now/
MIT License
1.26k stars 111 forks source link

Core Layout GraphCMS Duplication #350

Closed samuelcastro closed 3 years ago

samuelcastro commented 3 years ago

First of all, good job using next-plugin-preval I really liked it. But I noticed some duplication in the getCoreStaticProps and I saw you left a comment related. My questions is why don't you just do this: https://github.com/UnlyEd/next-right-now/pull/349

Vadorequest commented 3 years ago

Thanks! It's still in progress, it works fine in staging/production but is currently disabled in dev. I've spoken with the author who's making bugfixes and improvements, and I hope the developer experience will become better soon.

Vadorequest commented 3 years ago

Although, I'm pretty sure it can be optimized thanks to the new way the getServerSideProps function is returned. See https://github.com/UnlyEd/next-right-now/discussions/348

I haven't taken time to look into it but if you wish to look into it a PR is much welcome :)

samuelcastro commented 3 years ago

AFAIU, if apollo is loaded in the browser without the cache, then it'll need to re-fetch during the React reconciliation phase. I believe the cache is what's make the data available from the backend to the frontend, but I might be wrong, or it might be possible to do it another way. I'm not so experienced with the Apollo's internals so I preferred to duplicate code rather than create a bug/regression.

But, as you stated, since the data are statically available, it makes sense there would be a way to write the code only once.

We could add the static data into the cache: https://www.apollographql.com/docs/react/caching/cache-interaction/#writequery or https://www.apollographql.com/docs/react/caching/cache-interaction/#using-cachemodify

Vadorequest commented 3 years ago

I'm not using Apollo in production anymore (besides the NRN demo), I'm not so confident about recommending something like that without prior experience.

If you end up playing with it and come up with a recommendation/PR, then I might consider spending more time on it!

samuelcastro commented 3 years ago

https://github.com/UnlyEd/next-right-now/pull/349

Vadorequest commented 3 years ago

I'll take a look and run some tests, but yeah, it's seems both simple and shouldn't have any downside indeed!

samuelcastro commented 3 years ago

Fixed small issue: https://github.com/UnlyEd/next-right-now/pull/349/commits/50fb63b94afc8abfbc5f864e6f839ffc3dbda259

Vadorequest commented 3 years ago

Thanks, I've merged your PR! 🎉