Update react scripts

[tsmock]

This updates react-scripts from 4.0.3. to 5.0.1. Due to some issues with react-scripts, CRACO was added to work around an issue with svg images. CRACO did cause the main bundle size to increase significantly to 6 MB, and the main bundle size can reduced via lazy loading in the router (see #5914). There is more in-depth analysis in that PR on decreasing the initial size of all bundles needed for the initial render.

[HelNershingThapa]

Shouldn't the CRACO configuration file have been included in this PR?

[tsmock]

Yep. It looks like I messed up when I was cherry-picking the commit; I didn't run git add on the config file.

[HelNershingThapa]

Let's also include the fix https://github.com/hotosm/tasking-manager/pull/5721/commits/9ed1d0c6608723b4d8b7f2aa356a8be4eb47c76d for the testimonial image in this PR?

[HelNershingThapa]

@tsmock, would it be okay to merge the pull request and address the --stats flag in a subsequent update?

[tsmock]

Works for me. I'm currently checking to make certain that tests didn't get borked on my last rebase.

[tsmock]

I've removed the --stats and done a few (3) test runs. The tests have been passing.

[tsmock]

We might want to hold off on merging this -- I just fixed a bug in @placemarkio/geo-viewport w.r.t. webpack 5. The fix has been merged, but is not yet released.

If you run yarn build and get Module not found: Error: Default condition should be last one, that is the problem that is fixed in current geo-viewport main but is not yet released.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.1% Duplication

[tsmock]

geo-viewport released a fixed version, and I've updated it in this PR.

[HelNershingThapa]

The tests pass successfully, but the CircleCI process Run yarn test exists before generating the build with Exited with code exit status 1.

[tsmock]

Looking at https://github.com/jestjs/jest/issues/9324, it seems like there might be a test that is doing something wrong that might be causing the failure. I'm currently in the process of bisecting the test files, and I'm probably going to have to do that multiple times.

EDIT: It looks like the Cannot log after tests are done. Did you forget to wait for something async in your test? messages are probably the reason.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[varun2948]

Hi @tsmock what's the view of yours on transitioning to vite? Will it be a lot of work or something i don't know ? Just a query

[tsmock]

Doing it manually will be a lot of work. I looked into it a bit awhile ago (sometime last year). I don't remember why I dropped it (it might have been not wanting to have to do another massive PR while I had one in progress).

In my investigations, I found that someone wrote a script to eject to vite (https://github.com/bhbs/viject), so that will take care of the bulk of the work.

Things to note:

• We use flow in some locations. We can use flow comments and/or convert to something else (typescript, jsdoc, etc). I think I converted flow typing to flow comments in one of my PRs. I don't remember which one. It isn't that difficult, and still works with flow (we literally just have to wrap the flow typing with /* and */).
• We'll probably want to take advantage of SSR sometime (note: despite standing for "server-side rendering", it can be used to prerender static content to reduce resource usage on the client side).
• I think we'll have to/want to use a different library for routing. IIRC, vite has its own routing library. I might be thinking of a different project though.

[varun2948]

Hmmm the viject tools seems interesting, i will have a look into it soon. But yeah so for now are we be moving ahead with this PR ? and then onwards try to switch it to vite, as i think we will be using alot of vite in other product of HOT aswell so maybe something to be considered ? Vite has a plugin for routing i guess which recently changed it's name to Vike. At the moment for moving forward on the TM for vite or upgrading the react-scripts/craco what's your take , let me know further on this if its feasible or not?

[tsmock]

This was largely copied from #5721 so that we could build/run on Node 18+ without NODE_OPTIONS=--openssl-legacy-provider, so if #5721 is merged, that would remove the need for this PR.

TBH, I was using craco to work around problems in react-scripts (which is effectively no longer maintained). This is by no means the optimal path.

What I would do is the following:

• Merge #5721
• Run viject
• Fix problems
• Merge once things work "well enough" -- I think it would be a PITA to constantly rebase, since one of the changes done by viject is renaming files. I believe viject will work with react-scripts + craco.

Either way, I would strongly advocate moving away from react-scripts. It doesn't matter if it is next.js (Vercel), remix (Shopify), gatsby (Netlify), or vite.

With that said, it sounds like other HOT projects are using vite. So that is probably the winner.

[varun2948]

Hmmm sounds good will let ramya know on this aswell.

[ramyaragupathy]

Still relevant @varun2948 ?

[tsmock]

No, this PR is no longer relevant; the changes from this PR were in #5721.