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

Use dynamic router basename. #821

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

Hyperkid123
Copy link
Contributor

@Hyperkid123 Hyperkid123 commented Oct 27, 2021

Fixes invalid router basename when switching between bundles.

How to produce

Go to prod-beta env and switch between insights/openshift subscription on from chrome left nav. The URL will have an incorrect bundle segment.

Changes

  • use routerHelpers.dynamicBaseName function to assign router basename

Chrome is no longer disposing of the JS entry point of the remote module on each app/bundle switch. This means that "global" variables are not updated when we switch between modules common from the same remote container. Subscription has two modules, one for insights and one for openshift bundle.

Because the original routerHelpers.baseName is defined as a static constant in the entry file (webpack bundles it into the entry module because it is used in the remote entry AppEntry.js), it is not updated when the application changes context from /insights to /openshift. It always keeps the initial value based on the initial bundle. This lead to the router having an invalid basename if the bundle was switched via client-side routing.

@Hyperkid123 Hyperkid123 requested a review from cdcabrera October 27, 2021 07:34
@Hyperkid123 Hyperkid123 changed the base branch from master to ci October 27, 2021 12:46
@Hyperkid123 Hyperkid123 force-pushed the dynamic-basename branch 2 times, most recently from 9d3be4f to 8210559 Compare October 27, 2021 13:09
@cdcabrera cdcabrera added bug Something isn't working platform Contains, or is, platform specific work and issues 202112 project phase labels Oct 27, 2021
@cdcabrera
Copy link
Member

cdcabrera commented Oct 27, 2021

@Hyperkid123 just adjusted for

  • the commit message, and
  • moving dynamicBaseName from default props on redirect.js to allow for updating after component mount

Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

lgtm!

@cdcabrera cdcabrera merged commit 078f553 into RedHatInsights:ci Oct 27, 2021
@Hyperkid123 Hyperkid123 deleted the dynamic-basename branch November 1, 2021 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
202112 project phase bug Something isn't working platform Contains, or is, platform specific work and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants