fix: Use ObjectFetcher to retrieve objects #234
Conversation
mofojed
commented
Jan 29, 2024
mofojed
commented
- In Enterprise environment, there may be multiple connections. We should not be accessing the connection directly from the plugin for fetching objects, instead use the ObjectFetcher registered by the app
- Pass in the object metadata to fetch the object
- Needs feat: Add ObjectFetcher context and useObjectFetcher hook web-client-ui#1753
- In Enterprise environment, there may be multiple connections. We should not be accessing the connection directly from the plugin for fetching objects, instead use the ObjectFetcher registered by the app - Pass in the object metadata to fetch the object
- Use the Descriptor type instead of the raw Definition type
- update to use alpha version of web packages
mofojed
force-pushed
the
object-fetcher
branch
from
ae7770d to
21802cc
Compare
| @@ -31,20 +34,21 @@ interface DashboardPluginData { | |||
| type: string; | |||
| title: string; | |||
| id: string; | |||
| metadata: Record<string, unknown>; | |||
| widget: WidgetDescriptor; | |||
There was a problem hiding this comment.
I think this can just be VariableDescriptor from jsapi-types. In my generated jsapi types branch I just rebased and replaced WidgetDescriptor with VariableDescriptor. Unless you want them to stay separate. The definitions are the same except the jsapi types uses its | undefined | null for nullable instead of just | undefined
There was a problem hiding this comment.
Yea the only reason I kept them separate is the @deephaven/dashboard package has no dependencies on the JS API, and therefore cannot reference VariableDescriptor. And I think I'm alright with that - there's nothing saying we can't use dashboard with a different backend entirely... that being said, to smooth it out I'd probably argue that the WidgetDescriptor should require type and id, and then name should just be a friendly display name. Then the VariableDescriptor maps the id to the id or name field depending on the type of WidgetDescriptor (e.g. v: VariableDescriptor = { name: 'fiz', type: 'foo' } => w: WidgetDescriptor = { name: 'fiz', id: 'fiz', type: 'foo', variableType: 'name' }). Kludginess of having a VariableDescriptor defined and requires exactly one of id or name is already odd though and I didn't feel like adding another layer of indirection at this time made sense.
There was a problem hiding this comment.
Ya that's fair. I'll put it back in on my jsapi-types branch
