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

Unify backend/frontend/API code? #4

Closed
berekuk opened this issue Mar 16, 2022 · 6 comments
Closed

Unify backend/frontend/API code? #4

berekuk opened this issue Mar 16, 2022 · 6 comments

Comments

@berekuk
Copy link
Collaborator

berekuk commented Mar 16, 2022

Since getStaticProps and getServerSideProps code (and everything downstream of those) is not embedded in the frontend bundle, querying the DB directly from those is actually fine and recommended (see e.g. https://nextjs.org/docs/basic-features/data-fetching/get-server-side-props)

Unless I’m missing something, this means it’d be fine to unify all the code (frontend, backend and api) in a single Next.js app.

Secrets e.g. Postgres URL can be stored in env too (only the ones with NEXT_PUBLIC_ prefix are passed to the frontend, see https://nextjs.org/docs/basic-features/environment-variables).

If I understand correctly, the backend includes some kind of scheduler for updating the DB? (I haven't looked into the details yet). That can be handled with an API route and something like https://vercel.com/docs/concepts/solutions/cron-jobs (if we decide to use Vercel, see the next issue).

This was referenced Mar 16, 2022
@NunoSempere
Copy link
Collaborator

So there are two different things here:

  1. Create a monorepo with current services. This probably makes sense, though I had been putting it off.
  2. Using nextjs as an API. I am not excited about this. My sense is that it'd probably be fine, but eventually we'll want to do something which is a bit more complicated (sending reminders to users when a question updates, etc.), and nextjs wouldn't really support it.

That said, the API as it currently exists is pretty messy, and replacing it from scratch with a better graphql interface seems good.

@berekuk
Copy link
Collaborator Author

berekuk commented Mar 16, 2022

My sense is that it'd probably be fine, but eventually we'll want to do something which is a bit more complicated (sending reminders to users when a question updates, etc.), and nextjs wouldn't really support it.

I feel strongly enough about this that I'll try to argue otherwise. Sorry for the long rant :)

Some context:

  • I'm really paranoid about tight coupling with frameworks myself, I try to keep in mind the risk of framework lock-ins and losing the ability to implement features in the long run (I avoided Next.js for a long time on my last project and implemented an SPA with custom API server and hybrid server-side/client-side rendering before I admitted that Next.js does the same thing and does it exactly right in terms of flexibility)
  • I've been following the Next.js development for the last few years and they definitely understand where to keep it flexible; there are escape hatches everywhere
    • it's possible they'll eventually become less careful with escape hatches, but I don't think they're able to remove any without losing a significant portion of their userbase

Anyway, my points:

  1. Next.js pages/api magic is really thin and not really different from the raw expess.js server
  2. It's quite unhandy to add an extra http call from getStaticProps/getServerSideProps and it doesn't add much value (except that it proves that any page data is exposed via API, I guess that's kinda valuable)
  3. There's a corner case for long-running async/cron jobs, but those are better implemented as a separate backend process anyway; any HTTP API should be stateless and respond fast, Next.js or not; so we might eventually need separate backend node processes, but not for API

Here's what I have in mind:

  • src/ dir with all js code
  • src/pages/ for next.js pages
  • src/web/ for all React components
  • src/backend/ with all the backend code
  • getStaticProps/getServerSideProps from src/pages/* can call stuff from src/backend/
  • you could also have src/backend/worker.js for async/long-running jobs which is built and run separately from next start
  • you still could proxy requests from src/pages/api/ to a custom API server implemented in the same codebase through http-proxy if you ever need it (but I bet you won't, since there's no difference between Next.js and any express.js server)

I'll also note that even if we don't agree on this, it's important to keep all the code in the same codebase so that we don't have to think "do I need this function in Next.js or for my backend cron job? should I copy-paste?" (which really complicates things); since Next.js is smart with its bundling, there's no problem of pulling all the backend code into Next.js server, it still won't burden the frontend bundle, and it won't hurt the potential reuse of the same code for a different backend node process.

@berekuk
Copy link
Collaborator Author

berekuk commented Mar 16, 2022

It's quite unhandy to add an extra http call from getStaticProps/getServerSideProps and it doesn't add much value (except that it proves that any page data is exposed via API, I guess that's kinda valuable)

To be fair, another benefit is that it allows to run the frontend code without running the database. Which might be useful for small-time contributors.

But, extrapolating from the previous trends (and my own experience), contributors don't show up and add much value that often anyway.

@NunoSempere
Copy link
Collaborator

That makes sense. As you mentioned I was a bit paranoid about lock-in, but you mentioning

  • That there is not that much of a difference between the nextjs API and a small express server
  • There are still escape hatches
  • That only the NEXT_PUBLIC_ variables are passed to the frontend
  • And more generally you having looked into this more that I am

changes my mind because I'm mostly willing to trust your judgment here.

One last issue would be that with the code as currently structured, it's relatively easy for others to re-use it, whereas that might be a bit more difficult if we start using the nextjs api layer. But that's easily solvable by having better documentation/examples in a different folder.

@berekuk
Copy link
Collaborator Author

berekuk commented Mar 17, 2022

Cool.

I'll push monorepo branch to this repo soon (might happen tonight but it'll take some time to setup local env so it might take a day or two more) so that you can take a closer look at the code layout I have in mind before we merge it.

@berekuk berekuk mentioned this issue Mar 21, 2022
@berekuk
Copy link
Collaborator Author

berekuk commented Mar 26, 2022

Done by #8.

@berekuk berekuk closed this as completed Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants