HPC-9965 Upgrade react-router-dom to react-router v7.0.2
#480
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
Onitoxan
added
the
ready for review
unocha-hpc
approved these changes
Dec 9, 2024
Pl217
requested changes
Dec 11, 2024
unocha-hpc
approved these changes
Dec 16, 2024
Onitoxan
added
needs minor changes
https://reactrouter.com/upgrading/v6 no use of relative paths, `React.lazy` or `RouterProvide`, no changes needed
It was necessary to modify the dependency array of async-autocomplete-field for it to work
`react-router-dom` is no longer necessary, we can just import `react-router` for a more simplified package
This is a necessary step to being able to resolve import from `react-router/dom`
https://reactrouter.com/6.28.0/components/routes#routes In `hpc-cdm` there are many `<Routes />` elements, for the way is built it makes sense to let these `<Routes />` where they are instead of briging all of them to `main.tsx` since we have logic inside these `<Routes />`. In documentation they say that from `v6` and on, it is an 'uncommon practice', but for the way this was thought out, it would need a major rewrite or to let as it is for readability and maintenence ease purpose.
unocha-hpc
approved these changes
Dec 19, 2024
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.
Nature of PR
Description
(Link for upgrading documention to v7)
In this PR we focus on upgrading
react-router-domto its newest versionv7.0.2.During this upgrade this package has been replaced in favor ofreact-router. Of the mentioned necessary changes we had no need to do none of them since they didn't affect us really, except forv7_startTransitioninhpc-ftsadmin.In
async-autocomplete-field.tsxwe had anuseEffect()with the wrong variables inside of the dependency array. We decided to write all of them in order foreslintto not raise a warning of used variables inuseEffect()that were not in the dependency array. After further investigation this extra properties didn't cause a re-render what entered in conflict withv7_startTransition, since with this flagreact-routerusesuseTransition()instead ofuseState()what makes React categorize the importance of an action. The solution I provided, was going to my latest working branch ofhpc-ftsadminand took the values that I had in this dependencies array since I already encountered errors on this component and provided a fix.Beside this change there is another "important" change that is to change the
tsconfig.base.jsonand adding this new module resolution :We were using
nodethat anyways we would have had to update tonode16ornodenextsince we support modern versions of Node. But since we usedNxandwebpackI could activatebundlermode that give us more flexibility like not having to add all the file extensions of imports what would have resulted in a "big refactor". This was mainly done to allow us to import fromreact-router/dom, while usingnodewe could not resolve this import.I took the opportunity to implement a modern implementation of how they propose to create routers since
v6inhpc-ftsadminwhat will allow us to use the data API that will be necessary in the development of FTS Admin v2.How to test changes
To test this change I would recommend:
"moduleResolution": "bundler"doesn't raise any problems.hpc-ftsadminorhpc-cdm.ReactRouter7Adapteris well written.