[Maps] replace singleton inspectorAdapters with per map inspector adapter instance #31009
Conversation
| return false; | ||
| } | ||
|
|
||
| destroy() { |
There was a problem hiding this comment.
duplicate implementation of AbstractESSource.destroy not needed
nreese
added
v8.0.0
v7.2.0
Team:Geo
|
Pinging @elastic/kibana-gis |
💔 Build Failed |
thomasneirynck
left a comment
thomasneirynck
left a comment
There was a problem hiding this comment.
This PR looks bigger than it really is. Tested in chrome, works good.
Thanks!
|
|
||
| getInspectorAdapters() { | ||
| return this._inspectorAdapters; | ||
| } |
There was a problem hiding this comment.
So we need to introduce this method? Could a layer-instance keep a reference to the _inspectorAdapters.
I'd add this._inspectorAdapters = inspectorAdapters to the AbstractLayer constructor, which takes a second argument, just like AbstractSource
Not 100% sure, but I wouldn't expand the interface of sources, just so we can pull a reference to the inspectoradapter back up in the layers.
There was a problem hiding this comment.
Summing up our chat, I am going to leave as is since Layer does not need access to inspectorAdapters except in one isolated condition - when the VectorLayer creates LeftInnerJoin. In this case, VectorLayer never uses inspectorAdapters, it just needs access to inspectorAdapters to pass to the LeftInnerJoin constructor.
Adding inspectorAdapters to Layer constructor would spider web out since layers are created all over the place
💔 Build Failed |
…s, fix test jest test broken by import
💚 Build Succeeded |
…pter instance (elastic#31009) * [Maps] replace singleton inspectorAdapters with per map inspector adapter instance * pass inspectorAdapters to all source constructors in renderEditor * rename non_serializable_kibana_instances to non_serializable_instances, fix test jest test broken by import
nreese
added
the
release_note:skip
3 participants
This PR prepares the
inspectorAdaptersfor embeddables.inspectorAdapterscan not be a singleton because there could be multiple map instances in a Dashboard. Each embedded map instance must have its own inspectorAdapters instance.The PR puts
inspectorAdaptersin the store so its accessible by all connected components. Redux just uses react Context under the covers to pass state to connected components. Context allows non serializable objects. Using the store was a simpler solution then adding another Context wrapper aroundGisMap. I isolated the non serializable redux state in its own tree to minimize the location of non serializable state to a single location.