Skip to content

Commit

Permalink
Move react-scripts & node-sass to devDependencies
Browse files Browse the repository at this point in the history
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](facebook/create-react-app#11092 (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.
  • Loading branch information
monfresh committed Jun 16, 2021
1 parent f85e015 commit 699e032
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion dangerfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,6 @@ const checkYarnAudit = () => {
if (!danger.github || (danger.github && danger.github.pr.user.login !== 'dependabot-preview[bot]')) {
githubChecks();
fileChecks();
// checkYarnAudit();
checkYarnAudit();
cypressUpdateChecks();
}
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
"js-cookie": "^2.2.0",
"lodash": "^4.17.21",
"moment": "^2.29.1",
"node-sass": "^4.14.1",
"normalizr": "^3.6.1",
"numeral": "^2.0.6",
"path": "^0.12.7",
Expand Down Expand Up @@ -54,7 +53,6 @@
"react-router-dom": "^5.2.0",
"react-router-last-location": "^2.0.1",
"react-router-tabs": "^1.1.1",
"react-scripts": "^4.0.3",
"react-select": "^4.3.1",
"react-table": "^7.7.0",
"react-table-6": "^6.11.0",
Expand Down Expand Up @@ -150,7 +148,9 @@
"markdown-spellcheck": "^1.3.1",
"mime-types": "^2.1.31",
"mockdate": "^3.0.5",
"node-sass": "^4.14.1",
"prettier": "2.3.1",
"react-scripts": "^4.0.3",
"react-select-event": "^5.3.0",
"react-test-renderer": "^17.0.1",
"source-map-explorer": "^2.5.2",
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@
resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.12.11.tgz#c9a1f021917dcb5ccf0d4e453e399022981fc9ed"
integrity sha512-np/lG3uARFybkoHokJUmf1QfEvRVCPbmQeUQpKow5cQ3xWrV9i3rUHodKDJPQfTVX61qKi+UdYk8kik84n7XOw==

"@babel/helper-validator-identifier@^7.14.0", "@babel/helper-validator-identifier@^7.14.5":
"@babel/helper-validator-identifier@^7.14.5":
version "7.14.5"
resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.14.5.tgz#d0f0e277c512e0c938277faa85a3968c9a44c0e8"
integrity sha512-5lsetuxCLilmVGyiLEfoHBRX8UCFD+1m2x3Rj97WrW3V7H3u4RWRXA4evMjImCsin2J2YT0QaVDGf+z8ondbAg==
Expand Down

0 comments on commit 699e032

Please sign in to comment.