Move response/request and editor handler registration to uiExports #12766
Conversation
…stryProvider appropriately A bit of a tacky solution but once embeddable handlers PR is checked in, I can move this visualize specific code over to the visualize embeddable handler and pull it out of dashboard.
stacey-gammon
changed the title
|
Updated based on feedback from @spalger, doing this the right way, so no plugins have to reach into another plugin to import code manually. This has the potential to break other plugins, though we tried to sift through all of them to ensure no other areas required the new additions to the cc machine learning (@sophiec20) |
|
I don't spot any issues with the code, but it's hard to know what I'm checking for here. Even though I can see the issue that has a reference to this PR, it would be nice to give some description on how to test this. |
|
@tsullivan the only way to test it really is to try and use the editors/visualizations. When things build and the tests pass it's working. |
|
By the way, I'm assuming this fixes the UI Settings Service issue where it dies because the internal |
|
And I was able to test this branch with the change that was causing that issue and it does indeed fix it! Woohoo. Doubly LGTM. |
| import _ from 'lodash'; | ||
| import angular from 'angular'; | ||
| import defaultEditorTemplate from './default.html'; | ||
|
|
||
| import { VisEditorTypesRegistryProvider } from 'ui/registry/vis_editor_types'; | ||
|
|
There was a problem hiding this comment.
this might be problematic ....
right now someone might import the default editor (or any of the other things registered) into his code, which will register a duplicate with registryProvider ? (not sure does registryProvider allow duplicates ? what will happen in that case?) should either check for this scenario, or not export anything (so importing of this module is not possible, it should only be accessed thru registry
There was a problem hiding this comment.
Importing the module won't cause the code to execute twice. As long as the register call is only in one place it will be fine.
There was a problem hiding this comment.
Either way, there shouldn't be any reason to import the default editor, ideally consumers will only import the editors register and depend on all editors so that they can be pluggable. If consumers import a specific editor they aren't pluggable.
There was a problem hiding this comment.
same is true for requestHandlers and responseHandlers
|
LGTM |
|
@stacey-gammon I didn't backport this because it wasn't labeled as such. Let me know if I should |
No description provided.