[Shared UX Router Component] Fix context implementation #186532
Draft
rshen91 wants to merge 14 commits intoelastic:mainfrom
Draft
[Shared UX Router Component] Fix context implementation #186532rshen91 wants to merge 14 commits intoelastic:mainfrom
rshen91 wants to merge 14 commits intoelastic:mainfrom
Conversation
Contributor
Author
|
/ci |
Contributor
Author
|
/ci |
Contributor
Author
|
/ci |
⏳ Build in-progress, with failuresFailed CI Steps
Test Failures
History
To update your PR or re-run it, just comment with: |
Member
|
I think we can close this PR as we did the fix here: #204547 |
This file contains hidden or 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
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.
Summary
Issue raised in team slack. The pageName in the execution context remains the same on every route change. This component is not widely used so this was not caught until June 19.
I'm not sure if we should have this component since it might be dated - but open to discussion. Based on an initial look, it seems that many other places in code are using react-router vs this component and that might be a preferred alternative.
Main issue - there is a circular dependency that can be seen in core_system.ts. application is instantiated and then it's value is used for the kibana execution context. I'm not sure if the path forward is to change the dependencies of the Router to not need the execution context...or to move the Router out of the application and have it in chrome?
Checklist
Delete any items that are not applicable to this PR.