Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add ObjectFetcher context and useObjectFetcher hook #1753

Merged
merged 13 commits into from
Feb 2, 2024

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jan 29, 2024

  • Register a context that will fetch an object given the provided object metadata
  • Pass in a VariableDescriptor to fetch an object
    • On Enterprise, the VariableDescriptor object will be extended to include connection information (such as which query/session the variable is from)

BREAKING CHANGE:

  • useConnection is moved from jsapi-components package to app-utils package
    • Should only be used at the app level, as there could be multiple connections
  • WidgetDefinition has been renamed to WidgetDescriptor

- Register a context that will fetch an object given the provided object metadata
- Make the metadata parameter required for the useDeferredApi calls
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (ac40c6f) 46.06% compared to head (d258f74) 46.10%.

Files Patch % Lines
...es/dashboard-core-plugins/src/ChartPanelPlugin.tsx 0.00% 8 Missing ⚠️
...dashboard-core-plugins/src/panels/ConsolePanel.tsx 0.00% 5 Missing ⚠️
packages/dashboard/src/layout/useDashboardPanel.ts 0.00% 5 Missing ⚠️
...s/app-utils/src/components/ConnectionBootstrap.tsx 25.00% 3 Missing ⚠️
...ges/dashboard-core-plugins/src/GridPanelPlugin.tsx 0.00% 2 Missing ⚠️
...s/dashboard-core-plugins/src/PandasPanelPlugin.tsx 0.00% 2 Missing ⚠️
packages/jsapi-bootstrap/src/useObjectFetcher.ts 92.85% 2 Missing ⚠️
...ashboard-core-plugins/src/panels/IrisGridPanel.tsx 50.00% 1 Missing ⚠️
...kages/dashboard-core-plugins/src/useHydrateGrid.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1753      +/-   ##
==========================================
+ Coverage   46.06%   46.10%   +0.03%     
==========================================
  Files         626      627       +1     
  Lines       37596    37622      +26     
  Branches     9460     9470      +10     
==========================================
+ Hits        17319    17345      +26     
  Misses      20222    20222              
  Partials       55       55              
Flag Coverage Δ
unit 46.10% <57.97%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

packages/jsapi-bootstrap/src/useObjectFetcher.ts Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useObjectFetcher.ts Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useObjectFetcher.ts Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useObjectFetcher.ts Outdated Show resolved Hide resolved
- Added JSDocs and some tests
- Add VariableDescriptor type and fix typings of getObject methods
- Use VariableDescriptor type instead of defining a ObjectMetadata type
- Removing `fetch` from props - means panel should always be serializable
- Use VariableDescriptor name
- Move ConnectionContext to app-utils
- Clean up types around metadata
- Most annoyingly, have the `VariableDescriptor` which includes both the name and ID (so we have the metadata we need to display a table title), but then sanitize the object before we fetch from the API
- Get the naming for `widget` in the open panel event
packages/app-utils/src/components/ConnectionBootstrap.tsx Outdated Show resolved Hide resolved
packages/app-utils/tsconfig.json Outdated Show resolved Hide resolved
packages/dashboard-core-plugins/src/ChartPanelPlugin.tsx Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useDeferredApi.ts Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useObjectFetcher.test.ts Outdated Show resolved Hide resolved
packages/jsapi-bootstrap/src/useObjectFetcher.ts Outdated Show resolved Hide resolved
packages/plugin/src/PluginTypes.ts Outdated Show resolved Hide resolved
- Change up how typing is on the ObjectFetcher generic
  - Downside is now there's casting at the ObjectFetcherContext.Provider level - wonder if there's some other way to resolve it
@mofojed mofojed merged commit 2cd46ce into deephaven:main Feb 2, 2024
5 checks passed
@mofojed mofojed deleted the object-fetcher branch February 2, 2024 22:13
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants