Migrate to Parcel #57
Conversation
| @@ -1 +1,2 @@ | |||
| REACT_APP_GOOGLE_CLIENT_ID=706375540729-sqnnig7o0d0uqmvav0h8nh8aft6l55u3.apps.googleusercontent.com | |||
| API_BASE=https://hydrant.xvm.mit.edu | |||
There was a problem hiding this comment.
We probably need to use a separate xvm instance to do the server properly, unless we want to implement it all in CGI. @psvenk thoughts?
| @@ -0,0 +1,64 @@ | |||
| const { Reporter } = require('@parcel/plugin'); | |||
| const { fork } = require('child_process'); | |||
There was a problem hiding this comment.
This is a small plugin to relaunch the server whenever you update the code. It's much more consistent than something like nodemon and not too hard to read.
| @@ -32,7 +32,7 @@ function TypeSpan(props: { flag?: string; title: string }) { | |||
| <Image | |||
| alt={title} | |||
| boxSize="1em" | |||
| src={`img/${flag}.gif`} | |||
| src={flagURLs[flag]} | |||
There was a problem hiding this comment.
We can get the old static links back if needed but this (i.e. letting Parcel generate URLs for assets) is the way Parcel suggests because it works better w.r.t. caching.
| return 'Hello world!'; | ||
| }) | ||
|
|
||
| for (const file in data) { |
There was a problem hiding this comment.
This is probably going to change to something more robust eventually. I was thinking of migrating all the scraping stuff into the Node.js backend so we'd have a single codebase for everything; does that seem reasonable?
There was a problem hiding this comment.
i will as a matter of principle block this PR until more of how to develop is documented in the README.md, i think.
it doesn't have to be much, but enough that someone who doesn't care about the backend can spin up the frontend to work on it. the ideal would even be to explain how the backend works to someone who knows nothing about servers.
this is a big PR and can be split into several PRs:
- upgrading node to 18, which will need to touch the github workflows
- adding eslint, and fixing the codebase so there are no errors
- (while we're at it, get prettier… the formatting in these files is inconsistent)
- importing the images rather than referring to them by url
- setting up a fastify server, setting it up to serve the files properly, setting up a deployment workflow, testing it, making sure it works, and documenting all this
- (which can be done independently of ejecting from c-r-a! and probably should)
- moving the frontend to parcel (and touching the github workflows, etc)
i don't actually care if it gets split up or not, i'll leave that to other maintainers. my major blocker is that this needs to be documented better. (my minor blocker is that we need to get prettier and make sure all our files end with newlines…)
|
also thanks for the work!! |
|
Updated the docs a little but they're obviously not close to done. I'd like to be able to make the backend switch incrementally in a few separate PRs by the way; could you make a feature branch ( |
|
As you mentioned it would be a good idea to get Prettier set up and make the Fastify backend actually work in a separate PR, but I'd prefer to do that after this is merged to avoid merge conflicts. Also, it's possible (but a pretty big pain) to undo some of the changes that aren't strictly related to Parcel (e.g. the beginnings of the backend) and redo them in a PR without ejecting CRA. I decided to do them in one go because the backend's development lifecycle actually depends on Parcel too, and doing them separately would've been pretty annoying, but I can still undo it if needed. |
|
FYI I started some work on the Fastify server. Zod + Fastify is so much nicer to work with than Express! |
Apologies for the large change, but the majority of this diff is from the lockfile. I thought it would be a good idea to move off of Create React App since I need to refactor most of the build tooling anyway to make things work nicely with Node.js.
I understand if switching the build tooling entirely is too big a change to make, but we will need to sacrifice a lot of developer experience on the server-side code if we stay with CRA. This change makes hot reload on the frontend dramatically faster, and it obviously makes backend HMR much more doable.
I did consider using
ts-node-devin parallel with the current CRA infrastructure, but I don't think it would be nearly as nice for DX. If you don't like this change we can do that instead though.I'll highlight the parts of this diff that actually should get reviewed; the rest aren't very relevant.