Skip to content
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

Issue #7 Solution #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Issue #7 Solution #7

wants to merge 3 commits into from

Conversation

zacharynoel
Copy link
Owner

Changes

List Changes Introduced by this PR

  1. Changes to package.json and package-lock.json after updating all dependencies to their latest version.
  2. Update to AppRouter to wrap our Route components within a single Routes component. This was necessary after updating react-router-dom.
  3. Update to Login to use a navigation hook to redirect the user to the 'view-orders' page after logging in. This was also necessary after updating react-router-dom.

Purpose

Describe the problem or feature in addition to a link to the issues.

  • Link to issue: Update Dependencies Shift3/react-challenge-project-jan-2022#7
  • Description: The project had 3 low, 157 moderate, 287 high, and 64 critical vulnerabilities due to outdated dependencies. It is beneficial to keep those packages updated when possible, but it takes some careful consideration with the process since updates can introduce breaking changes.

Approach

How does this change address the problem?

The changes in this update include all of our packages being updated to their latest version, and all breaking changes fixed. Vulnerabilities have also been reduced to 1 remaining, which requires manual review (running npm audit fix does not fix this vulnerability, as it is a dependency of our react-scripts package, which is already fully up-to-date).

Pre-Testing TODOs

What needs to be done before testing

  • Run npm install for all dependencies to be updated based on the versions listed in package.json.

Testing Steps

How do the users test this change?

  1. [Time sensitive testing]: run npm outdated to confirm that all packages are up-to-date. Of course, this will change as time passes and new versions are released after today (up-to-date as of June 26, 2022).
  2. [Time sensitive testing]: run npm audit to confirm that the vulnerabilities have been reduced to 1 remaining (the one that requires manual review, as mentioned above). Much like the previous step, this will change as time passes and new vulnerabilities are found.
  3. Run the app and attempt to login. Make sure you are successfully redirected to the 'view-orders' page. This confirms the fixes that were required in Login and AppRouter.

Learning

Describe the research stage

Below are the detailed steps that I took throughout the full update process.

  1. Run npm outdated to get a list of packages, our current versions, and the latest versions available. This helps gauge how significant the changes may be with updating (are any of the updates going to a new major version?)
  2. Out of the 11 packages, 7 will be updated to a new major version.
  3. Run npm audit to gauge how many vulnerabilities are found.
  4. Results show 3 low, 157 moderate, 287 high, and 64 critical
  5. To start, I decided to update the 4 packages that do not require updating to a new major version. I did each of these 1 at a time, running the app after each update to check for any breaking changes.
  6. Updating those 4 packages went smoothly, no breaking changes were introduced so far.
  7. I then decided to repeat that process of updating each package one at a time for the remaining 7 packages, knowing that those are being updated to new major versions and may very well introduce breaking changes.
  8. While updating the remaining packages 1 at a time, I ran into my first breaking change after updating react-router-dom.
  9. The error message was clear, stating that Route is only ever to be used as the child of Routes. This was introduced in v6 of react-router-dom, and the update was from v5.2.0. After finding a post describing this change (linked below), I wrapped all Route components in AppRouter within a single Routes component and changed the component property for each Route to element.
  10. After making the changes above, the project was running again, but logging in was still broken.
  11. After some research on the error given in our Login component, I knew we needed to update Login to be a functional component and use hooks for navigating.
  12. At first I find information about using the useHistory() hook, but then notice that has been replaced by the useNavigate() hook.
  13. After looking at the documentation for React Router on useNavigate(), I update Login to be a functional component and use the useNavigate() hook to direct the user to '/view-orders' after logging in.
  14. After the above solution, everything is now working properly and I can move forward with updating the remaining packages.
  15. Updating the remaining 3 packages goes smoothly and running npm outdated no longer shows any packages that need to be updated.
  16. Running npm audit has now reduced the vulnerability count significantly. Results now show 5 moderate, 11 high. 15 of those vulnerabilities are tied to lodash, and 1 is tied to nth-check.
  17. Next, I run npm audit fix which is able to fix those 15 lodash vulnerabilities.
  18. What remains now is 1 vulnerability for nth-check, which requires manual review. nth-check is listed as a dependency of react-scripts after running npm audit again.
  19. Lastly, I do a final run through of the app to make sure there are no breaking changes that still need to be addressed.

Links to blog posts, patterns, libraries or addons used to solve this problem

  1. Stack Overflow post that made it clear wrapping with Routes was needed as of react-router-dom v6
  2. Stack Overflow post on updating the property in Route from component to element
  3. Stack Overflow post describing the need to use useHistory() hook
  4. Blog explaining that the useHistory() hook is actually replaced now, and that I should use the useNavigate() hook instead
  5. React Router official documentation on using the useNavigate() hook

Closes Shift3#7

…ent (necessary change after updating react-router-dom)
…se a navigation hook to redirect the user to the /view-orders page after logging in (necessary change after updating react-router-dom)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Dependencies
1 participant