Runtime: Add provider and access hook for location service #90759
Conversation
This file contains 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
aocenas
requested review from
gabor,
joshhunt,
ashharrison90 and
jackw
and removed request for
a team
leventebalogh
approved these changes
Aug 1, 2024
| children, | ||
| }) => { | ||
| return <LocationServiceContext.Provider value={service}>{children}</LocationServiceContext.Provider>; | ||
| }; |
There was a problem hiding this comment.
It would be nice to add a basic test case for the hook and the provider in LocationService.test.ts(x).
aocenas
changed the title
giorgioatmerqury
pushed a commit
to cybermerqury/grafana
that referenced
this pull request
Aug 21, 2024
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.
This is part of the effort to make the LocationService non singleton as part of the Sidecar project, see #88797.
This right now just adds a provider for the LocationService and adds it into the React tree that renders app plugins. This way app plugins can get the location service from the context instead of relying on the singleton. This means we can later inject different location providers for 2 separate apps and they won't clash on the URL. You can see the plan here https://github.com/grafana/grafana/pull/88797/files#diff-b1d459ae81cb312ba78da28cfe277013ba08a6787c0f5dcc42ac142f06dadd6cR181 with locationService with MemoryHistory being injected to the second app rendering tree.
Currently the injected locationService is the same as global singleton so nothing should change for grafana itself or any app.
This is also required for this Scenes PR that makes it's URL handling not reliant on the locationService singleton.