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

Move from create-react-app to parcel + babel #3041

Draft
wants to merge 91 commits into
base: master
Choose a base branch
from

Conversation

imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Apr 2, 2024

To follow #2875, #2973, #2975, #3054, #3179, #3313
Resolves #2133

When testing this branch, npx jest --clearCache may be necessary if the frontend tests aren't working.


This change is Reviewable

@imnasnainaec imnasnainaec marked this pull request as ready for review August 14, 2024 14:27
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much less ugly then I was expecting...

Reviewed 6 of 23 files at r1, 1 of 1 files at r2, 22 of 22 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)


.gitignore line 13 at r3 (raw file):

# Production
build

It wouldn't hurt to leave this during transition - although we should catch any sins in code review.


jest.config.json line 2 at r3 (raw file):

{
  "collectCoverageFrom": [

If there was anything in here that required extra learning you might want to see if any comments are honored here and add them.


scripts/jestTransform/babelTransform.js line 1 at r3 (raw file):

"use strict";

A comment explaining the goal and purpose of this transform might help someone in the future. It is in the jestTransform folder, but folder names are too subtle in my experience when trying to understand code.

Copy link
Collaborator Author

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 26 of 29 files reviewed, all discussions resolved (waiting on @jasonleenaylor)


jest.config.json line 2 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

If there was anything in here that required extra learning you might want to see if any comments are honored here and add them.

Though our tool chain is not set up to allow comments in JSON files (though I haven't investigated how easily it could be), Jest config specs have a "//" option just for this purpose. And there's docs are quite good on this matter, so I'm not sure what comment is worthwhile here beyond pointing to their docs.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

@imnasnainaec imnasnainaec marked this pull request as draft September 20, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file frontend refactor Size: L Large sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecated middleware options
2 participants