Migrate to Parcel #57

101arrowz · 2023-09-26T21:30:29Z

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-dev in 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.

101arrowz · 2023-09-26T21:31:27Z

.env

@@ -1 +1,2 @@
 REACT_APP_GOOGLE_CLIENT_ID=706375540729-sqnnig7o0d0uqmvav0h8nh8aft6l55u3.apps.googleusercontent.com
+API_BASE=https://hydrant.xvm.mit.edu


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?

101arrowz · 2023-09-26T21:32:50Z

scripts/parcel-dev-reporter.js

@@ -0,0 +1,64 @@
+const { Reporter } = require('@parcel/plugin');
+const { fork } = require('child_process');


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.

101arrowz · 2023-09-26T21:35:01Z

src/components/ActivityDescription.tsx

@@ -32,7 +32,7 @@ function TypeSpan(props: { flag?: string; title: string }) {
      <Image
        alt={title}
        boxSize="1em"
-        src={`img/${flag}.gif`}
+        src={flagURLs[flag]}


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.

101arrowz · 2023-09-26T21:53:39Z

src-server/server.ts

+  return 'Hello world!';
+})
+
+for (const file in data) {


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?

cjquines

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…)

cjquines · 2023-09-27T02:55:34Z

also thanks for the work!!

101arrowz · 2023-09-27T23:44:26Z

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 (node-backend or something) so I can PR to that without screwing up the main branch in the meanwhile? That should also make the incomplete docs less of an issue.

101arrowz · 2023-09-27T23:53:29Z

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.

101arrowz · 2023-09-28T02:01:25Z

FYI I started some work on the Fastify server. Zod + Fastify is so much nicer to work with than Express!

cjquines

unblocking, will leave someone else to approve

dtemkin1 · 2024-12-05T18:36:42Z

Hi! Is there any desire to rebase this PR on main? Migrating off of CRA would be really nice, but if not, I can open a new pull request

Working with CRA is a bit of a pain and there's been work to move off of it. I didn't update #57 since that involves the creation of a backend server too (which involves setting up XVM and all that) and this is more of an instant change, although rebasing that PR too would be good (maybe over IAP when ppl have more time?). This PR moves off of CRA and moves to Vite. This also involves having to recreate some of the tooling integrated into CRA, such as an eslint config (a lot of the diffs in component files were issues flagged by tseslint that weren't being checked before). I didn't really have to change the workings of any components, which was nice. This issue also resolves all the security vulnerabilities found when running `npm install`, and also closes #89. Doing this would probably make anything we want to do in the future, such as adding tests (which can be done with [Vitest](https://vitest.dev/)) or integrating a backend server (which can be done using [a plugin](https://github.com/axe-me/vite-plugin-node)), a bit easier.

gabrc52 · 2025-03-02T03:29:07Z

@dtemkin1 Is this PR still relevant?

It has been open for 2 years now

dtemkin1 · 2025-03-02T19:24:09Z

Uhhh I think this PR was the start of making a backend for Hydrant, we should prob talk about it at today's meeting I suppose

cjquines · 2025-03-03T03:05:06Z

this PR should be unnecessary now that we've moved to vite

dtemkin1 · 2025-03-03T18:16:34Z

I guess we can close it, would be good to continue discussing creating a backend (or we might be able to create similar features through a tighter integration with Fireroad?)

cjquines · 2025-03-03T21:52:04Z

we should definitely use the fireroad backend instead when possible

101arrowz added 2 commits September 26, 2023 10:24

prepare for ejection

adeb1dc

Refactor to parcel

18898d5

101arrowz commented Sep 26, 2023

View reviewed changes

fix static assets

b2a02d3

101arrowz commented Sep 26, 2023

View reviewed changes

cjquines requested changes Sep 27, 2023

View reviewed changes

draft updated docs, remove cron scripts

2e11856

cjquines reviewed Sep 28, 2023

View reviewed changes

cjquines self-requested a review September 28, 2023 03:01

cjquines approved these changes Sep 28, 2023

View reviewed changes

cjquines self-requested a review September 28, 2023 03:01

psvenk self-requested a review October 2, 2023 22:57

dtemkin1 mentioned this pull request Dec 8, 2024

move off of cra to vite #96

Merged

dtemkin1 closed this Mar 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Parcel #57

Migrate to Parcel #57

101arrowz commented Sep 26, 2023 •

edited

Loading

101arrowz Sep 26, 2023

101arrowz Sep 26, 2023

101arrowz Sep 26, 2023 •

edited

Loading

101arrowz Sep 26, 2023

cjquines left a comment •

edited

Loading

cjquines commented Sep 27, 2023

101arrowz commented Sep 27, 2023

101arrowz commented Sep 27, 2023

101arrowz commented Sep 28, 2023

cjquines left a comment

dtemkin1 commented Dec 5, 2024

gabrc52 commented Mar 2, 2025

dtemkin1 commented Mar 2, 2025

cjquines commented Mar 3, 2025

dtemkin1 commented Mar 3, 2025

cjquines commented Mar 3, 2025

		@@ -1 +1,2 @@
		REACT_APP_GOOGLE_CLIENT_ID=706375540729-sqnnig7o0d0uqmvav0h8nh8aft6l55u3.apps.googleusercontent.com
		API_BASE=https://hydrant.xvm.mit.edu

		@@ -0,0 +1,64 @@
		const { Reporter } = require('@parcel/plugin');
		const { fork } = require('child_process');

Migrate to Parcel #57

Migrate to Parcel #57

Conversation

101arrowz commented Sep 26, 2023 • edited Loading

101arrowz Sep 26, 2023

Choose a reason for hiding this comment

101arrowz Sep 26, 2023

Choose a reason for hiding this comment

101arrowz Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

101arrowz Sep 26, 2023

Choose a reason for hiding this comment

cjquines left a comment • edited Loading

Choose a reason for hiding this comment

cjquines commented Sep 27, 2023

101arrowz commented Sep 27, 2023

101arrowz commented Sep 27, 2023

101arrowz commented Sep 28, 2023

cjquines left a comment

Choose a reason for hiding this comment

dtemkin1 commented Dec 5, 2024

gabrc52 commented Mar 2, 2025

dtemkin1 commented Mar 2, 2025

cjquines commented Mar 3, 2025

dtemkin1 commented Mar 3, 2025

cjquines commented Mar 3, 2025

101arrowz commented Sep 26, 2023 •

edited

Loading

101arrowz Sep 26, 2023 •

edited

Loading

cjquines left a comment •

edited

Loading