VisTypesRegistryProvider => VisualizationsPlugin interface - Draft #43459
Conversation
| export const VisTypesRegistryProvider = { | ||
| register: (obj: VisType) => visTypes.set(obj.name, obj), | ||
| get: (key: string) => visTypes.get(key), | ||
| getAll: () => new Map(visTypes), |
There was a problem hiding this comment.
Maybe change this to
getAll: () => visTypes.values(),or
getAll: () => [...visTypes.values()],| interface VisTypesRegistryAccessors { | ||
| byName: { [typeName: string]: VisType }; | ||
| } | ||
| export type VisTypesRegistry = Map<string, VisType>; |
There was a problem hiding this comment.
Maybe remove this line.
And instead add
export interface NewPlatformVisTypePluginContract {
registerVisType: (visType: VisType) => void;
get: (name: string) => VisType;
getAll: () => IterableIterator<VisType>;
}
mattkime
added
non-issue
|
Pinging @elastic/kibana-app-arch |
mattkime
changed the title
mattkime
changed the title
mattkime
changed the title
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
retest |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
| VisTypesRegistryProvider.register(gaugeVisTypeProvider); | ||
| VisTypesRegistryProvider.register(goalVisTypeProvider); | ||
| async function registerItems() { | ||
| const $injector = await chrome.dangerouslyGetActiveInjector(); |
There was a problem hiding this comment.
@ppisljar chrome.dangerouslyGetActiveInjector(); doesn't return in the browser tests and some 80 tests fail. Perhaps we need an alternative way of registering these - what the best way to do it for a karma test?
There was a problem hiding this comment.
hmm ... interesting .... i had a feeling we did this in other places that were tested by karma and it was working, will try to look into it later today if time allows.
| @@ -22,7 +22,7 @@ import { i18n } from '@kbn/i18n'; | |||
| import chrome from 'ui/chrome'; | |||
| import { VisRequestHandlersRegistryProvider as RequestHandlersProvider } from 'ui/registry/vis_request_handlers'; | |||
| import { VisResponseHandlersRegistryProvider as ResponseHandlerProvider } from 'ui/registry/vis_response_handlers'; | |||
| import { VisTypesRegistryProvider } from 'ui/registry/vis_types'; | |||
| import { VisTypesRegistryProvider as visTypes } from 'ui/registry/vis_types'; | |||
There was a problem hiding this comment.
you should get the registry from visualizations plugin
|
|
||
| constructor() { | ||
| this.filters = new FiltersService(); | ||
| this.types = new TypesService(); | ||
| this.visualizations = new Map(); | ||
| } | ||
|
|
||
| public setup() { | ||
| return { | ||
| filters: this.filters.setup(), | ||
| types: this.types.setup(), |
There was a problem hiding this comment.
its actually this types service that we should replace with 'visualizations' you created here.
types currently point to ui/registry/vis_types, but we should replace it with the map you created and provide register/get/getAll functions
| @@ -64,7 +64,7 @@ describe('VegaVisualizations', () => { | |||
| uiSettings: $injector.get('config'), | |||
| }; | |||
|
|
|||
| visualizations.types.VisTypesRegistryProvider.register(() => | |||
| visualizations.types.VisTypesRegistryProvider.register( | |||
There was a problem hiding this comment.
then we should replace all usages to the new one:
visualization.types.register(...)
| @@ -18,7 +18,7 @@ | |||
| */ | |||
|
|
|||
| import './_saved_vis'; | |||
| import { VisTypesRegistryProvider } from 'ui/registry/vis_types'; | |||
| import { VisTypesRegistryProvider as visTypes } from 'ui/registry/vis_types'; | |||
There was a problem hiding this comment.
and fix all imports pointing to the old registry to import the new one
| @@ -43,7 +43,7 @@ export function VisualizeListingController($injector, createNewVis) { | |||
| const kbnUrl = $injector.get('kbnUrl'); | |||
| const savedObjectClient = Private(SavedObjectsClientProvider); | |||
|
|
|||
| this.visTypeRegistry = Private(VisTypesRegistryProvider); | |||
| this.visTypeRegistry = VisTypesRegistryProvider.getAll(); | |||
There was a problem hiding this comment.
as well as every place we read from the registry
Summary
Summarize your PR. If it involves visual changes include a screenshot or gif.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers