-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vite + Upgrade arcgis to 4.30 + esri-loader => @arcgis/loader #691
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This is a duplicate of #689 to merge in develop |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upgrade arcgis to 4.30. Update CRA -> Vite and Esri-loader to @arcgis/loader
Description
To update arcgis to 4.30 we had to change esri-loader with @arcgis-loader and to do that we couldn't depend anymore on react-app-rewired so we changed the bundler to vite.
Updates:
React updated to 17 (This change was made to support the Vite change)
Esri-loader library replaced by @arcgis/core This change was made because the esri-loader library was deprecated in arcgisjs 4.29.
Create react app (react-app-rewired) replaced by Vite. This change was made as react-app-rewired was "lightly maintained" as their page says. This made very difficult any further changes. With vite we have a simple packing system that we will be able to evolve. A further step could be using Next.js but that will require many more changes.
Removed unnecesary dependencies
Icons are now imported no more as:
import { ReactComponent as IconArrow } from 'icons/arrow_right.svg';
but now:
import IconArrow from 'icons/arrow_right.svg?react';
Important:
I have updated the environment variables on vercel, everything that started before REACT_APP_ should be VITE_APP_ now.
Vite creates the bundle on the dist directory so I added a vercel.json that overrides the folder build directory with the following content:
Once this is deployed to production we will be able to change the folder build directory directly on Vercel and we won't need this file anymore.
Race condition problems with react 17
There were some race condition problems in React 17 with the system that we have to registry the redux modules based on http://nicolasgallagher.com/redux-modules-and-code-splitting/
Basically the data was tried to be loaded before the module was registered so it wasn't ready and empty once it was registered.
The temporary solution was to load the affected modules directly on the store file avoiding the redux splitting.
My recommendation is to change to redux-toolkit all the redux system and use slices so we have a more robust redux without sacrificing data splitting.
This also could bring us the opportunity to integrate it with react-query and improve the cache on each query.