Conversation
ExploreManager builds the PersesPlugin passed to PluginLoaderComponent without version, registry or baseURL, so the module federation remote entry resolved to /plugins/<module>/mf-manifest.json. With api_prefix set, that URL is not served and every explorer plugin fails to load. Pass the module version and registry through from the plugin metadata, and let the plugin runtime fall back to the assets path the remote plugin loader was configured with instead of a hardcoded /plugins. Signed-off-by: Yash Lunawat <yash.lunawat@rtp.vc>
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.
Description
Fixes perses/perses#4455
When Perses is served under an
api_prefix, for example/perses, every explorer plugin fails withPluginLoaderComponent: Error loading plugin XXXExplorer from module XXX. Themf-manifest.jsonrequest goes to/plugins/Tempo/mf-manifest.jsoninstead of/perses/plugins/Tempo~0.59.0/mf-manifest.json.Two things combine to cause it:
ExploreManagerbuilds thePersesPluginit hands toPluginLoaderComponentwith onlynameandmoduleName. The moduleversionandregistryare available oncurrentPlugin.modulebut get dropped, so the remote entry path loses its version and registry suffix.registerRemoteinPluginRuntimefalls back to a hardcoded/pluginswhen a plugin carries nobaseURL.remotePluginLoaderis already configured with the right assets path, since the app passesapi_prefixinto it, but nothing shares that with plugins loaded throughPluginLoaderComponent, which bypassesPluginRegistry.Datasource, panel and query plugins are unaffected because they go through
remotePluginLoader.importPluginModule, which threadsbaseURL,versionandregistrycorrectly.Changes
ExploreManagerpassesversionandregistryfrom the plugin's module metadata, matching whatremotePluginLoaderdoes.PluginRuntimekeeps the assets base URL the plugin loader was configured with and uses it as the fallback instead of the hardcoded/plugins. A plugin's ownbaseURLstill wins.remotePluginLoaderregisters its resolvedpluginsAssetsPathwith the runtime.I kept
window.PERSES_APP_CONFIGout of it, which the issue offers as an alternative, since this repository should not depend on product-specific globals. Reading the path the loader was already given keeps the configuration in one place and fixes any consumer ofPluginLoaderComponent, not only the explorer.Worth noting: the issue is filed on perses/perses and quotes
ui/explore/...paths, but those packages moved here in perses/perses#3659, so this is where the fix belongs. The field names in the issue's suggestion are also slightly off,moduleis itself the metadata object, so the values aremodule.versionandmodule.registry.Verification
Added a test in
PluginRuntime.test.tsasserting the manifest entry uses the loader's configured base URL, and one inExploreManager.test.tsxasserting the plugin object carries the module's version and registry. Both fail without the change; the first fails with exactly the URL from the bug report. All five workspace suites pass (1216 tests), along with lint, format check, type check, build andmake checklicense.Screenshots
No visual change. The fix is in the URL used to fetch plugin manifests.
Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes