Skip to content

Conversation

jnumainville
Copy link
Contributor

@jnumainville jnumainville commented Jun 27, 2025

Will also need deephaven/deephaven-plugins#1201 for the plugin side, but this implements a hook that pulls the necessary mapping for element plugins

@jnumainville jnumainville changed the title 19146 element plugin feat: DH-19146: ElementPlugin mapping hook Jul 7, 2025
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.64%. Comparing base (7785090) to head (e783149).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2477      +/-   ##
==========================================
+ Coverage   44.62%   44.64%   +0.02%     
==========================================
  Files         759      760       +1     
  Lines       42550    42564      +14     
  Branches    10693    10696       +3     
==========================================
+ Hits        18989    19004      +15     
+ Misses      23550    23549       -1     
  Partials       11       11              
Flag Coverage Δ
unit 44.64% <100.00%> (+0.02%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jnumainville jnumainville requested review from mofojed and Copilot July 8, 2025 23:06
@jnumainville jnumainville marked this pull request as ready for review July 8, 2025 23:06
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds support for extracting element-specific plugins by introducing a new hook, utility function, type definitions, and associated tests.

  • Introduces useElementPluginMapping hook to build a mapping of element names to their React components from the plugin context.
  • Extends PluginUtils and PluginTypes with ElementPlugin support and the getElementPluginMapping helper.
  • Adds unit tests for the new hook and getElementPluginMapping function.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/plugin/src/useElementPluginMapping.tsx New hook to retrieve element plugin mapping
packages/plugin/src/index.ts Exports the new hook
packages/plugin/src/UseElementPluginMapping.test.ts Tests for the new hook
packages/plugin/src/PluginUtils.tsx Added getElementPluginMapping and logging
packages/plugin/src/PluginUtils.test.tsx Tests for getElementPluginMapping
packages/plugin/src/PluginTypes.ts Defined ElementPlugin type and isElementPlugin
Comments suppressed due to low confidence (2)

packages/plugin/src/UseElementPluginMapping.test.ts:1

  • [nitpick] Test file name casing (UseElementPluginMapping.test.ts) differs from the hook file (useElementPluginMapping.tsx). Consider aligning test file name to useElementPluginMapping.test.ts for consistency.
import { renderHook } from '@testing-library/react-hooks';

packages/plugin/src/PluginUtils.tsx:89

  • React is not imported in this file, but React.ComponentType is used. Add import { type ComponentType } from 'react'; or import React from 'react'; at the top.
): Map<string, React.ComponentType<unknown>> {

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small things to clean up

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple other things I noticed after reviewing the deephaven-plugins side.

@jnumainville jnumainville requested a review from mofojed July 11, 2025 15:08
@jnumainville jnumainville force-pushed the 19146_element_plugin branch from 7be48fb to 003027c Compare July 14, 2025 19:38
@jnumainville jnumainville requested a review from mofojed July 14, 2025 19:38
@jnumainville jnumainville enabled auto-merge (squash) July 14, 2025 19:39
mofojed
mofojed previously approved these changes Jul 14, 2025
@jnumainville jnumainville merged commit bc16dbd into deephaven:main Jul 14, 2025
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2025
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.

2 participants