Vltava - Dynamic Component Discovery via Registry #1126
Conversation
…nto registry-container-service-vltava
…nto registry-container-service-vltava
skylerjokiel
requested review from
tylerbutler,
anthony-murphy,
ChumpChief,
curtisman and
vladsud
| } | ||
|
|
||
| const generateFactory = () => { | ||
| const containerComponentsDefinition: IContainerComponentDetails[] = [ |
There was a problem hiding this comment.
Schema to define what components exist in the container
|
|
||
| public getNewTabTypes(): ITabsTypes[] { | ||
| const response: ITabsTypes[] = []; | ||
| this.internalRegistry.getFromCapabilities("IComponentHTMLVisual").forEach((e) => { |
There was a problem hiding this comment.
This is the interesting part. The DataModel gets the registry and searches for capabilities that support IComponentHTMLVisual only to display.
| @@ -37,8 +37,15 @@ export class TabsComponent extends PrimedComponent implements IComponentHTMLVisu | |||
| } | |||
|
|
|||
| protected async componentHasInitialized() { | |||
| const registry = await this.context.hostRuntime.IComponentRegistry.get(""); | |||
There was a problem hiding this comment.
getting the registry from the runtime and passing it into the model.
| factory: Promise.resolve(ClickerInstantiationFactory), | ||
| capabilities: ["IComponentHTMLVisual"], | ||
| friendlyName: "Clicker", | ||
| fabricIconName: "NumberField", |
There was a problem hiding this comment.
i think these properties should be on the object returned by the factory. as it seems to me that ideally these are exposed by the component's factory so it is self describing, rather than defined by the app.
There was a problem hiding this comment.
I had the same thought process. There are pros for each direction. It's similar to the "type" defined in the mapping. If we put it in the factory we have to resolve the factory. If we don't then there is no common place.
I have it living outside for now because I'm don't want to be committed to updating all the factories that I'm using. That is a pattern I'm exploring though.
| * Licensed under the MIT License. | ||
| */ | ||
|
|
||
| import * as React from "react"; |
There was a problem hiding this comment.
it'd be nice to split reviews next time, as there is a lot of code modified here.
There was a problem hiding this comment.
sorry about that :(
| @@ -37,8 +37,15 @@ export class TabsComponent extends PrimedComponent implements IComponentHTMLVisu | |||
| } | |||
|
|
|||
| protected async componentHasInitialized() { | |||
| const registry = await this.context.hostRuntime.IComponentRegistry.get(""); | |||
There was a problem hiding this comment.
"" [](start = 79, length = 2)
i don't like the use of empty string here, why not just exposes IComponentRegistryDetails off the IComponentRegistry, and inspect this.context.hostRuntime.IComponentRegistry rather than having to get a fake registry and inspect that.
There was a problem hiding this comment.
100% agree but currently you can't set the default registry. I logged a bug to fix this:
#1138
There was a problem hiding this comment.
uggh. yeah. forgot about that. it has to do with the _scheduler.
This PR explores the Container Registry as a discovery mechanism for the Tabs Component.
Before this change the Tabs Component hard coded all it's components and it was required that the Tabs Component brought these components along or they were defined in the container.
Now, when constructing the Container we pass in a Registry that we treat as an IComponent. Components can request the registry and query for the available components and capabilities of that components. This is currently schematized and pretty specific to Vltava but This can definitely be extensible. Now when new components are added to the Container Registry these components will be available for other components to consume.
This isn't an end goal but a good stopping place in the progression.
Example:
https://www.wu2.prague.office-int.com/waterpark/damaged-sister1?chaincode=@fluid-example/vltava@0.13.0-skjokiel-1
Also in this pr