Fix many JS vulns, fix tests #1775
Conversation
| @@ -227,7 +227,7 @@ function remeasure() { | |||
|
|
|||
There was a problem hiding this comment.
NOTE: all application code changes are as a result of yarn run lint --fix and not manually performed
Stephen2
changed the title
|
Need this library to be updated so I can close JS vulns that are blocking our build pipeline |
|
Not necessary. Maintainer is busy but responsive |
Includes updating and installing missing/wrong peer deps
2.x broke tests so left a 1.x
Stephen2
force-pushed
the
sp-update-css-select
branch
from
0c4533c to
8c6cb1b
Compare
| }, | ||
| "devDependencies": { | ||
| "@babel/core": "^7.18.2", |
There was a problem hiding this comment.
Could you briefly explain why all those newly added libs are necessary? I can see that @babel/plugin-syntax-flow, @babel/plugin-transform-react-jsx etc. were added. If there is a good reason behind it, I think it is probably ready to be merged then.
There was a problem hiding this comment.
Sure, good question.
After updating the eslint related packages that already exist, yarn shows warnings about peer deps:
warning " > [email protected]" has unmet peer dependency "@babel/plugin-syntax-flow@^7.14.5".
warning " > [email protected]" has unmet peer dependency "@babel/plugin-transform-react-jsx@^7.14.9".
warning " > [email protected]" has unmet peer dependency "@babel/core@^7.0.0".
So to resolve these warnings I've installed the peer deps as well using yarn add -D
There was a problem hiding this comment.
Ok, the last thing that I am afraid of is updating the css-select and css-tree deps since it will directly affect the users of the library. Is it fixing any vulnerabilities? If not, maybe it should be added in some other PR in order not to mix things for lib developers with those of lib users. Wdyt?
There was a problem hiding this comment.
Updating css-select is important to close vulnerability (this is the original reason I need to make this PR)
The dep tree is:
react-native-svg>css-select>nth-check
There was a problem hiding this comment.
css-tree we could undo, but I was thinking if tests pass why not update it while I'm doing all this other work
There was a problem hiding this comment.
@WoLewicki please let me know your final decision here.
There was a problem hiding this comment.
Sorry for the delay, I think we can do it here, so it is ready to be merged 🎉 Thanks for your contribution!
Summary
Upgrades a ton of dependencies, closes JS vulns, gets lint & test working as intended. Only barely touches application code, due to TypeScript version upgrade and prettier upgrade.
Also makes
yarn run testwork again, by ignoringExamplefolders which aren't working right now.Test Plan
I ensured that
tsc,lintandtestpass