Upgrade react scripts+react to latest versions #1930
Conversation
github-actions
bot
added
the
needs label triage
cmdcolin
added
internal
and removed
needs label triage
Codecov Report
@@ Coverage Diff @@
## main #1930 +/- ##
==========================================
- Coverage 60.11% 59.70% -0.42%
==========================================
Files 467 470 +3
Lines 21877 22195 +318
Branches 5097 5097
==========================================
+ Hits 13152 13252 +100
- Misses 8423 8641 +218
Partials 302 302
Continue to review full report at Codecov.
|
|
close but still fails some odd lint rules on CI that dont fail locally "Definition for rule '@typescript-eslint/no-redeclare' was not found @typescript-eslint/no-redeclare" no mention of this in our codebase...random suspect is eslint-config-airbnb-typescript so may need to upgrade or remove it |
|
this appears to generate a storybook build memory failure on CI works locally on my machine, but on CI it produces this
|
|
The normal full repo build also appears to fail...killed after two hours running It would be good to get the upgrades but needs some work to make sure CI passes |
…the dependencies of useEffect Hook" Failed to compile. src/Loader.tsx Line 47:9: The 'ipcRenderer' logical expression could make the dependencies of useEffect Hook (at line 69) change on every render. Move it inside the useEffect callback. Alternatively, wrap the initialization of 'ipcRenderer' in its own useMemo() Hook react-hooks/exhaustive-deps src/StartScreen.tsx Line 192:9: The 'sortedSessions' conditional could make the dependencies of useEffect Hook (at line 216) change on every render. To fix this, wrap the initialization of 'sortedSessions' in its own useMemo() Hook react-hooks/exhaustive-deps
…stead of strictly v16
…yfill for TextEncoder/TextDecoder
|
This PR actually now passes CI. Previously it was timing out CI but only when run build under node 10, but running under node 14 works, so I updated to that. I don't think we have a strict need to make our CI run under node 10, so this should be ok. It is a bit of a crazy thing to make upgrades like this, but I think they can be important to avoid getting stale in the long run |
|
Note that, if merged, you may have to rm -rf node_modules and re-run yarn. If not, you may (i got this at least) get this jest error when running tests |
|
Note that this is a large diff because I remove airbnb-typescript It contains many rules that in a large number of cases, we just ignored. Therefore, this removes a lot of ignores, and it also fixes some usage of curlies that the previous PR (#1925 ) didn't catch maybe since this is upgraded to a newer eslint version |
|
I think you have to keep the devDependency and update it to 26.6.0. This is a special case of a local devDependency overriding the monorepo-wide one, see #1300 (comment). |
|
interesting, I thought I had tried basically that (I had ^26 in dev dependency instead of explicitly 26.6.0) tried clearing that up and seems like it worked |
|
marking as non-draft |
cmdcolin
force-pushed
the
upgrade_react_scripts
branch
from
93dcf89 to
ab5af65
Compare
|
@garrettjstevens I added skipLibCheck to the jbrowse-cli also now..I found that the jbrowse-cli gave errors with @types/react when I updated root package.json @types/react to 17. This skipLibCheck is an option that is true in our root config Short explainer on stackoverflow https://stackoverflow.com/questions/52311779/usage-of-the-typescript-compiler-argument-skiplibcheck It seems like it relates to declare.d.ts files...I think this should be ok for cli...the cli doesn't even use react hehe |
|
xref 058017b |
|
this should be good to go now :) |
In order to avoid letting our dependencies get stale on us, I tried upgrading react-scripts and react...
This I think had been attempted before, but I tried again just to see...seemed to work