move react-scripts & node-sass to devDependencies

[monfresh]

Description

Last week, our builds started failing because of a vulnerability in
react-scripts and node-sass, and we have a Danger rule to run
yarn audit on the packages in the dependencies section.

The vulnerabilities haven't been fixed yet, and so to allow us to merge
PRs, we temporarily disabled the checkYarnAudit function in
dangerfile.ts.

While looking at the GitHub issues for these vulnerabilities that were
linked in our SEV-4 incident Google Doc, I came across this
interesting comment
that says that in the facebook/create-react-app package,
react-scripts should be listed in devDependencies, not
dependencies.

That got me thinking whether the packages in our dependencies section
really belong there. AFAIK, sass is used in development and then gets
compiled to CSS when the client is built. It doesn't get used at
runtime. Similarly, react-scripts seems to be a development tool we
use to run yarn build | eject | start | test.

After putting both node-sass and react-scripts in devDependencies,
I deployed the app using our review bot and everything seems fine.

This allows us to turn the yarn audit check back on.

https://my-milmove-pr-6819.mymove.sandbox.truss.coffee
https://admin-milmove-pr-6819.mymove.sandbox.truss.coffee
https://office-milmove-pr-6819.mymove.sandbox.truss.coffee

Reviewer Notes

Is there anything you would like reviewers to give additional scrutiny?

Setup

Add any steps or code to run in this section to help others prepare to run your code:

echo "Code goes here"

Code Review Verification Steps

• If the change is risky, it has been tested in experimental before merging.
• Code follows the guidelines for Logging
• The requirements listed in Querying the Database Safely have been satisfied.
• Any new migrations/schema changes:
  • Follow our guidelines for zero-downtime deploys (see Zero-Downtime Deploys)
  • Have been communicated to #g-database
  • Secure migrations have been tested following the instructions in our docs
• There are no aXe warnings for UI.
• This works in Supported Browsers and their phone views (Chrome, Firefox, IE, Edge).
• Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
• User facing changes have been reviewed by design.
• Request review from a member of a different team.
• Have the Jira acceptance criteria been met for this change?

References

• Jira story for this change
• this article explains more about the approach used.

Screenshots

If this PR makes visible UI changes, an image of the finished UI can help reviewers and casual
observers understand the context of the changes. A before image is optional and
can be included at the submitter's discretion.

Consider using an animated image to show an entire workflow instead of using multiple images. You may want to use GIPHY CAPTURE for this! 📸

Please frame screenshots to show enough useful context but also highlight the affected regions.

[robot-mymove]

	Warnings
⚠️	It looks like you updated the Cypress package dependency in one of two required places. Please update it in both the root package.json and the cirlcleci-docker/milmove-cypress/ folder's separate package.json
⚠️	Please add the JIRA issue key to the PR title (e.g. MB-123)

Generated by 🚫 dangerJS against 699e032

[github-actions]

Bundle difference

Old size	New size	Diff
46.41 MB	46.41 MB	0 B (0.00%)

[felipe-lee]

Based on looking at the comment thread, the proposal, and the original pr that made the change to make it a dependency instead of a dev dependency, I don't really see an issue with this. The only thing I wish was explained better in there was why it being a dev dependency affects the promise polyfill being a runtime instead of build time dependency. I guess that'd be my concern, though given that IE11 is EOL next year, maybe it doesn't matter.

[monfresh]

Also, that comment is from facebook/create-react-app, and AFAICT, we aren't using create-react-app itself, just react-scripts.

[github-actions]

Bundle difference

Old size	New size	Diff
46.4 MB	46.4 MB	0 B (0.00%)