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

3 of 4 - refactor(router): sw-625 move to router v6 #989

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

cdcabrera
Copy link
Member

@cdcabrera cdcabrera commented Oct 17, 2022

What's included

  • refactor(router): sw-625 move to router v6

Notes

  • the routing move and related PRs
    • create a generated list of pre-sorted product configs on app load. this is prep for loading actual product configurations
    • eliminate the need to have a large independent routing configuration that defines every route. instead we now use a route parameter see :productPath. This parameter is "kind-of" superfluous due to the use of a noticeably quicker way of loading product configuration by handling window.location.pathname directly within the new routerContext.js hooks and routeHelpers.js
    • removes the use of react context in favor of simple hooks that
      • initialize the product config lookup with useSetRouteDetail and
      • provide product configuration based on a route parameter/config lookup, using product configuration details and/or their closest match based on pathname...
      • partial product names typed into the url will return a product now. The caveat is, it's what the GUI/algorithm determines to be the closest string match which doesn't always align to "human". This can be disabled if we determine it's causing problems
    • simplifies components, code... always good

How to test

Coverage and basic unit test check

  1. update the NPM packages with $ yarn
  2. $ yarn test
  3. confirm tests come back clean

Check the build

  1. update the NPM packages with $ yarn
  2. $ yarn build
  3. confirm tests come back clean

Example

...

Updates issue/story

swatch-625

related #1048 #1045

@cdcabrera cdcabrera added platform Contains, or is, platform specific work and issues 202212 project phase labels Oct 17, 2022
@cdcabrera cdcabrera changed the title Review react router v6+ swatch-625 Review react router v6+ Oct 26, 2022
@cdcabrera cdcabrera changed the title swatch-625 Review react router v6+ SWATCH-625 Review react router v6+ Oct 26, 2022
@cdcabrera cdcabrera changed the title SWATCH-625 Review react router v6+ SWATCH-625: Review react router v6+ Oct 26, 2022
@cdcabrera cdcabrera added the blocked Prevented from continuing label Nov 10, 2022
@cdcabrera
Copy link
Member Author

cdcabrera commented Dec 8, 2022

As of 20221207 rebased and updated. Confirmed working both locally, and through the proxy against stage-beta.

We'll need to optimize how the route configs are being pulled since there looked like lag coming through the page loads, but that could also be associated with the proxy. The route configs don't change, once loaded, so caching them on an extended cache cycle might become a thing

@cdcabrera cdcabrera added hold and removed blocked Prevented from continuing labels Jan 24, 2023
@cdcabrera cdcabrera changed the title SWATCH-625: Review react router v6+ refactor(router): sw-625 move to router v6 Feb 8, 2023
@cdcabrera cdcabrera changed the title refactor(router): sw-625 move to router v6 3 of 4 - refactor(router): sw-625 move to router v6 Feb 14, 2023
cdcabrera added a commit to cdcabrera/curiosity-frontend that referenced this pull request Feb 23, 2023
cdcabrera added a commit to cdcabrera/curiosity-frontend that referenced this pull request Feb 23, 2023
cdcabrera added a commit to cdcabrera/curiosity-frontend that referenced this pull request Feb 23, 2023
@cdcabrera cdcabrera marked this pull request as ready for review February 24, 2023 02:10
@cdcabrera cdcabrera removed the hold label Feb 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2023

Codecov Report

Merging #989 (a053fcc) into dev (79fbda6) will decrease coverage by 0.14%.
The diff coverage is 92.23%.

❗ Current head a053fcc differs from pull request most recent head 37189ba. Consider uploading reports for the commit 37189ba to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #989      +/-   ##
==========================================
- Coverage   93.65%   93.52%   -0.14%     
==========================================
  Files         121      123       +2     
  Lines        4052     4138      +86     
  Branches     1633     1679      +46     
==========================================
+ Hits         3795     3870      +75     
- Misses        240      251      +11     
  Partials       17       17              
Impacted Files Coverage Δ
...components/authentication/authenticationContext.js 100.00% <ø> (+3.03%) ⬆️
src/config/product.openshiftContainer.js 80.95% <ø> (ø)
src/config/product.openshiftDedicated.js 80.76% <ø> (ø)
src/config/product.openshiftMetrics.js 80.00% <ø> (ø)
src/config/product.rhacs.js 85.71% <ø> (ø)
src/config/product.rhel.js 90.69% <ø> (ø)
src/config/product.rhods.js 85.71% <ø> (ø)
src/config/product.rhosak.js 83.72% <ø> (ø)
src/config/product.satellite.js 94.11% <ø> (ø)
src/config/routes.js 100.00% <ø> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79fbda6...37189ba. Read the comment docs.

src/components/productView/productView.js Show resolved Hide resolved
src/components/i18n/i18n.js Outdated Show resolved Hide resolved
* build, routing package, module ref, spelling
* configs, sorted products, aliases, simplify routes
* locale, OpenShift strings
* AppEntry, remove router component
* app, locale loading checks
* helpers, add memoize, object freeze
* authenticationContext, remove nav history push
* i18n, minor check to avoid reload
* productView, useRouteDetail hook update
* productViewMissing, add useNavigate, RouteDetail hooks
* redux, viewReducer, appTypes for storing route ref
* router, simplify, useSetRouteDetail for config load
* routerContext, useRouteDetail, wrap location, navigate
* routerHelpers, restructure getRouteConfigByPath, memo
@cdcabrera
Copy link
Member Author

/retest

@cdcabrera cdcabrera merged commit ca55f6c into RedHatInsights:dev Feb 24, 2023
cdcabrera added a commit that referenced this pull request Mar 2, 2023
* build, routing package, module ref, spelling
* configs, sorted products, aliases, simplify routes
* locale, OpenShift strings
* AppEntry, remove router component
* app, locale loading checks
* helpers, add memoize, object freeze
* authenticationContext, remove nav history push
* i18n, minor check to avoid reload
* productView, useRouteDetail hook update
* productViewMissing, add useNavigate, RouteDetail hooks
* redux, viewReducer, appTypes for storing route ref
* router, simplify, useSetRouteDetail for config load
* routerContext, useRouteDetail, wrap location, navigate
* routerHelpers, restructure getRouteConfigByPath, memo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
202212 project phase platform Contains, or is, platform specific work and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants