Upgrade react-scripts to v4.0.x

jackcmeyer commented 4 years ago

React scripts was recently upgraded to v4.0.0. This issue is to track and finish migrating HospitalRun to react-scripts v4.0.x. Dependabot couldn't do this on its own due to a failing build: https://github.com/HospitalRun/hospitalrun-frontend/pull/2437.

Things to consider:

npm start should start the app
npm test should successfully run the test suite
npm run lint should continue to report linting errors.

SamuelQZQ commented 3 years ago

@jackcmeyer I'm new here, let me have a try.

I found that the react-script v4.0x requires jest v26.6.0. But it seems we have failed in upgrade jest a few times. Do you have any information about this?

doubleppereira commented 3 years ago

Actually react-script v4.0 has jest already you don't need to have that dependency anymore..I was taking a look as well and it seems that after the update I found that we would need to ignore some eslint rules in sake of typescript ones.

'no-shadow': 'off',
    '@typescript-eslint/no-shadow': ['error'],
    // note you must disable the base rule as it can report incorrect errors
    'no-use-before-define': 'off',
    '@typescript-eslint/no-use-before-define': ['error'],

Even like this some tests are failing maybe some other rules or even the tests them selves I notice per example that ViewLabs.test looked like it had some race condition going on there but didn't have much time to investigate

nobrayner commented 3 years ago

I've started working on this after trying it out on the Convert to RTL branch.

Currently, I am having a lot of issues with the tests, and I don't seem to know enough to reason about them. Namely a lot of:

TypeError: Right-hand side of 'instanceof' is not callable

      at new ReadableState (node_modules/sublevel-pouchdb/node_modules/readable-stream/lib/_stream_readable.js:105:14)
      at ReadStream.Readable (node_modules/sublevel-pouchdb/node_modules/readable-stream/lib/_stream_readable.js:139:25)
      at new ReadStream (node_modules/sublevel-pouchdb/lib/index.js:289:12)
      at ReadStream (node_modules/sublevel-pouchdb/lib/index.js:286:12)
      at EventEmitter.emitter.readStream.emitter.createReadStream (node_modules/sublevel-pouchdb/lib/index.js:261:14)
      at PouchDB.LevelPouch.api._changes (node_modules/pouchdb-adapter-leveldb-core/lib/index.js:1102:42)
      at Changes$1.Object.<anonymous>.Changes$1.doChanges (node_modules/pouchdb/lib/index.js:1569:28)
      at node_modules/pouchdb/lib/index.js:1508:12
      at Object.validate (node_modules/pouchdb/lib/index.js:3805:3)
      at Changes$1.Object.<anonymous>.Changes$1.validateChanges (node_modules/pouchdb/lib/index.js:1504:34)
      at new Changes$1 (node_modules/pouchdb/lib/index.js:1465:10)
      at PouchDB.Object.<anonymous>.AbstractPouchDB.changes (node_modules/pouchdb/lib/index.js:2326:10)
      at processNextBatch (node_modules/pouchdb-abstract-mapreduce/lib/index.js:610:28)
      at updateViewInQueue (node_modules/pouchdb-abstract-mapreduce/lib/index.js:672:12)
      at node_modules/pouchdb-abstract-mapreduce/lib/index.js:578:14
      at node_modules/pouchdb-mapreduce-utils/lib/index.js:92:29
      at node_modules/pouchdb-abstract-mapreduce/lib/index.js:25:12

And then the real kicker, that completely kills the test run:

src/__tests__/patients/view/ViewPatient.test.tsx
C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:154
      var evt = document.createEvent('Event'); // Keeps track of whether the user-provided callback threw an error. We
                         ^

TypeError: Cannot read property 'createEvent' of null
    at Object.invokeGuardedCallbackDev (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:154:26)
    at invokeGuardedCallback (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:292:31)
    at flushPassiveEffectsImpl (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:22853:9)
    at unstable_runWithPriority (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\scheduler\cjs\scheduler.development.js:653:12)
    at runWithPriority$1 (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:11039:10)
    at flushPassiveEffects (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom.development.js:22820:12)
    at Object.<anonymous>.flushWork (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom-test-utils.development.js:876:10)
    at Immediate.<anonymous> (C:\_Source\hospitalrun-frontend-nobrayner\node_modules\react-dom\cjs\react-dom-test-utils.development.js:887:11)

EDIT: Got some help (@mpeyper) and it seems to be related to tests hovering around the 5s mark for test running time. jest.setTimeout(10000) resolves in this case. Hopefully this can be removed as part of the conversion to RTL #2516

nobrayner commented 3 years ago

I may revisit this after the RTL conversion, to see if it makes a difference. At the moment, 80+ tests fail for various reasons. And the number changes each time all the tests are run...

doubleppereira commented 3 years ago

I may revisit this after the RTL conversion, to see if it makes a difference. At the moment, 80+ tests fail for various reasons. And the number changes each time all the tests are run...

Yes I think the way the tests are being tested might have some race conditions happening so maybe with that conversion to RTL would solve that and would make upgrade to react 17 easier as well. I managed to have around 46 tests failing but I still have those errors that you mentioned above

HospitalRun / hospitalrun-frontend

Upgrade react-scripts to v4.0.x #2454