Skip to content

Upgrade React Router #1110#1896

Open
louise-davies wants to merge 10 commits into
react-query-5from
react-router-upgrade
Open

Upgrade React Router #1110#1896
louise-davies wants to merge 10 commits into
react-query-5from
react-router-upgrade

Conversation

@louise-davies

@louise-davies louise-davies commented Mar 25, 2026

Copy link
Copy Markdown
Member

Description

Biggest refactors are:

  • refactoring datagateway-dataview so that PageRouting is the "top level" component that gets imported via App, and PageContainer becomes a "layout route" that uses Outlet to render the actual route contents and matchPath to turn on/off different UI elements
  • refactoring many tests as they can't use history anymore
  • default sort/filtering broke in tests (but not in dev mode from what I could see?). Added an extra check to wait for the location to update with the default search params before performing the search. At this point it would likely easier to remove default sort/filtering functionality from Table and instead just link/route with the default params provided - but that's a larger refactor I didn't want to mix up in here

Otherwise just "standard" refactoring as according to the migration guides

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage
  • Test with SciGateway and check routing works

Agile board tracking

Closes #1110

@codecov

codecov Bot commented Mar 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.37833% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.78%. Comparing base (063f69b) to head (88ae16a).

Files with missing lines Patch % Lines
...gateway-common/src/homePage/homePage.component.tsx 25.00% 3 Missing ⚠️
packages/datagateway-common/src/main.tsx 0.00% 1 Missing ⚠️
packages/datagateway-common/src/utils.ts 50.00% 1 Missing ⚠️
...tagateway-dataview/src/page/redirect.component.tsx 93.33% 1 Missing ⚠️
...s/table/isis/isisInvestigationsTable.component.tsx 94.44% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           react-query-5    #1896   +/-   ##
==============================================
  Coverage          96.78%   96.78%           
==============================================
  Files                219      221    +2     
  Lines              30906    30909    +3     
  Branches            4587     4615   +28     
==============================================
+ Hits               29911    29916    +5     
+ Misses               990      988    -2     
  Partials               5        5           
Flag Coverage Δ
common 95.38% <93.97%> (-0.01%) ⬇️
dataview 98.62% <99.78%> (+0.02%) ⬆️
download 97.47% <100.00%> (-0.01%) ⬇️
search 95.65% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- either stop removing the default sort where is not needed any more and instead just test the column we're trying to sort is sorted
- or add some checks to ensure the default sort is actually applied before continuing the test
@louise-davies louise-davies marked this pull request as ready for review March 26, 2026 12:45
@louise-davies louise-davies marked this pull request as draft March 26, 2026 15:59
@louise-davies louise-davies marked this pull request as ready for review March 27, 2026 10:02

@joshdimanteto joshdimanteto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

2 participants