Inject UI plugins into injectedMetadata #31840
Conversation
joshdover
added
Team:Core
|
Pinging @elastic/kibana-platform |
💔 Build Failed |
joshdover
force-pushed
the
np-ui-plugins
branch
from
d2f7992
to
e9314e6
Compare
💚 Build Succeeded |
src/core/public/injected_metadata/injected_metadata_service.test.ts
Outdated
Show resolved
Hide resolved
joshdover
force-pushed
the
np-ui-plugins
branch
from
e9314e6
to
c173fc4
Compare
💚 Build Succeeded |
joshdover
force-pushed
the
np-ui-plugins
branch
3 times, most recently
from
ff7da46
to
2c5d2b1
Compare
💔 Build Failed |
💔 Build Failed |
|
@azasypkin @restrry This is ready for review again. The main change is now I'm exposing the entire PluginManifest to the client-side code. This involved moving some type information around to be accessible to both client-side and server-side code and renaming some of the exposed interfaces. |
💔 Build Failed |
| @@ -17,6 +17,7 @@ | |||
| * under the License. | |||
| */ | |||
|
|
|||
| import { DiscoveredPlugin } from 'src/core/server'; | |||
There was a problem hiding this comment.
Is this a TypeScript feature to allow imports from the root of the project like this?
There was a problem hiding this comment.
Hmm yeah I think my editor did that automatically, I'll make it relative.
💚 Build Succeeded |
|
One problem I see here is that it appears like more information is being injected in the browser and thus exposed to users than is necessary. Specifically I'd be concerned about file system path information being sent. To prevent the possibility of someone adding more information to plugin discovery in the future and not realizing it will get exposed to the browser, we should whitelist only the properties that are strictly necessary. |
Good point. For solving this, I prefer to have core deciding what should and should not be exposed to the browser and then making that clear to consumers. How about changing the /** Safe to expose to browser */
export interface DiscoveredPlugin {
readonly id: PluginName;
readonly configPath: ConfigPath;
readonly requiredPlugins: ReadonlyArray<PluginName>;
readonly optionalPlugins: ReadonlyArray<PluginName>;
}
/** Safe to use on server */
export interface DiscoveredPluginInternal extends DiscoveredPlugin {
readonly path: string;
}
export interface PluginServiceStart {
contracts: Map<PluginName, unknown>;
uiPlugins: {
public: Map<PluginName, DiscoveredPlugin>;
internal: Map<PluginName, DiscoveredPluginInternal>;
}
} |
|
I think the terminology is a little wonky. I hate there is a concept of I think I'm OK with that for now though. |
|
I debated that a few times as well, but I think |
💚 Build Succeeded |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
|
@epixa Did you have any more feedback on this one? |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
This modifies the PluginService to expose a Map of UI plugins and their manifests, in topological order by their dependencies. That data is then exposed to injectedMetadata via the ui_render_mixin in the legacy platform, and read back out via the new platform's client-side InjectedMetadataService, preserving the topological order.
joshdover
added
the
release_note:skip
This modifies the PluginService to expose a Map of UI plugins and their manifests, in topological order by their dependencies. That data is then exposed to injectedMetadata via the ui_render_mixin in the legacy platform, and read back out via the new platform's client-side InjectedMetadataService, preserving the topological order.
Summary
Phase 1 of #18874
Blocked by #32578This modifies the
PluginServiceto expose a Map of UI plugins and their manifests, in topological order by their dependencies. That data is then exposed to injectedMetadata via theui_render_mixinin the legacy platform, and read back out via the new platform's client-sideInjectedMetadataService, preserving the topological order.In future PRs, I'll use this data to load plugin bundles, initialize, and start plugins.