Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Add Craco and Prettier #1271

Merged
merged 5 commits into from Mar 11, 2021
Merged

Add Craco and Prettier #1271

merged 5 commits into from Mar 11, 2021

Conversation

TaiWilkin
Copy link
Contributor

@TaiWilkin TaiWilkin commented Mar 4, 2021

Overview

Move to the standard Craco/Prettier configuration we have in other projects, allowing for partially automated code formatting. Upgrade to React-Scripts 4.0.2 and React 16.

Connects #1266, #1252

Notes

Fast-reloading is enabled by default in CRA 4, but while it is correctly compiling the updated code it is not auto-refreshing. This might be due to Vagrant settings. Turning off the fast reloading allows for the hot-module reloading, which restores the previous behavior of auto-refreshes in the React app. We should continue to look into how to enable the fast refresh within the Vagrant environment.

Testing Instructions

  • On this branch, run vagrant ssh and vagrant update
  • Run ./scripts/lint and confirm no necessary changes are found
  • Run ./scripts/server and confirm that the website behavior hasn't changed
  • Make a change in a file and save. The app should reload as before.

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@TaiWilkin TaiWilkin changed the title WIP Add Craco and Prettier Mar 4, 2021
@TaiWilkin TaiWilkin changed the title Add Craco and Prettier WIP: Add Craco and Prettier Mar 4, 2021
@TaiWilkin TaiWilkin changed the title WIP: Add Craco and Prettier Add Craco and Prettier Mar 5, 2021
@TaiWilkin TaiWilkin requested a review from jwalgran March 5, 2021 14:39
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

Nice work sticking with this and completing a successful overhaul of our client side build system. I was able to successfully update and browse the app.

We are taking a small, hopefully temporary step back by switching to full page refreshes, but the workflow benefits of prettier are, in my opinion, worth it. We have discussed the possibility of running projects without Vagrant, and I was able to boot the services on my host Docker, but failed to browse either 6543 or 8081. Getting that sorted out may allow us to turn on fast refresh.

I would like to hold back on merging this until we deploy a release to production. We have a lot of features pending and want to avoid increasing the already large surface area of changes.

src/app/.prettierrc Show resolved Hide resolved
src/app/package.json Show resolved Hide resolved
@jwalgran
Copy link
Contributor

I am sorry for merging a dependabot PR before finishing this. The yarn.lock collision is a little confusing.

I merged this branch with master and resolved the yarn.lock conflicts favoring the higher versions in develop. Afterward I was able to update and run successfully from an empty node_modules folder, so I think dropping this yarn.lock file into your working directory when you rebase on develop should work.

yarn.lock.zip

@jwalgran jwalgran removed their assignment Mar 11, 2021
Images which are in the Public folder and are imported into CSS are no
longer correctly importing following the React-Scripts update. Moves
the images into the Styles/Images folder to allow access.
Adds Craco and Prettier config. Updates some dependencies to work
with the new libraries, including notably React-Scripts and React.
The update to React-Scripts introduces fast-reloading; therefore
the previous add-on setup for hot-reloading has been removed.

Prettier settings ported from our similar projects.
Fix items which are caught by lint but cannot be automatically fixed by
Prettier.
When running `./scripts/server`, files will now be autoformatted with
Prettier. This commit is the results of running formatting on all js/jsx
files in the client app.
@TaiWilkin TaiWilkin merged commit 4c8d91b into develop Mar 11, 2021
@TaiWilkin TaiWilkin deleted the tw/use-craco branch March 11, 2021 18:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants