Skip to content

Commit

Permalink
fix: Remove unused ref forwarded to all dashboard panels (#1451)
Browse files Browse the repository at this point in the history
This ref doesn't seem to be used by anything and prints dev warnings if
any panel is a functional component that doesn't use `React.forwardRef`.
I couldn't find anywhere we actually use the ref and it looks like it
was a workaround for golden-layout adding the ref as default props to
every component. The way we use golden-layout, I don't think it's even
possible to attach a ref to the component in a way that a plugin could
access that ref.
  • Loading branch information
mattrunyon authored Aug 22, 2023
1 parent 2a4ec0d commit 938aa07
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 26 deletions.
14 changes: 4 additions & 10 deletions packages/dashboard/src/DashboardLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,29 +118,23 @@ export function DashboardLayout({
componentDehydrate
);

function renderComponent(props: PanelProps, ref: unknown) {
// Cast it to an `any` type so we can pass the ref in correctly.
// ComponentType doesn't seem to work right, ReactNode is also incorrect
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const CType = componentType as any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const PanelWrapperType = panelWrapper as any;
function wrappedComponent(props: PanelProps) {
const CType = componentType;
const PanelWrapperType = panelWrapper;

// Props supplied by GoldenLayout
// eslint-disable-next-line react/prop-types
const { glContainer, glEventHub } = props;
return (
<PanelErrorBoundary glContainer={glContainer} glEventHub={glEventHub}>
{/* eslint-disable-next-line react/jsx-props-no-spreading */}
<PanelWrapperType {...props}>
{/* eslint-disable-next-line react/jsx-props-no-spreading */}
<CType {...props} ref={ref} />
<CType {...props} />
</PanelWrapperType>
</PanelErrorBoundary>
);
}

const wrappedComponent = React.forwardRef(renderComponent);
const cleanup = layout.registerComponent(name, wrappedComponent);
hydrateMap.set(name, componentHydrate);
dehydrateMap.set(name, componentDehydrate);
Expand Down
22 changes: 9 additions & 13 deletions packages/dashboard/src/DashboardPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ import type {
} from '@deephaven/golden-layout';
import PanelManager from './PanelManager';

/**
* Alias for the return type of React.forwardRef()
*/
export type ForwardRefComponentType<P, R> = ForwardRefExoticComponent<
PropsWithoutRef<P> & RefAttributes<R>
>;

/**
* Panel components can provide static props that provide meta data about the
* panel.
Expand All @@ -36,6 +29,14 @@ export interface PanelStaticMetaData {
}

/**
* Alias for the return type of React.forwardRef()
*/
type ForwardRefComponentType<P, R> = ForwardRefExoticComponent<
PropsWithoutRef<P> & RefAttributes<R>
>;

/**
* @deprecated Use `PanelComponentType` instead and add generic types to forwardRef call.
* Panels defined as functional components have to use React.forwardRef.
*/
export type PanelFunctionComponentType<P, R> = ForwardRefComponentType<P, R> &
Expand All @@ -49,12 +50,7 @@ export type WrappedComponentType<
export type PanelComponentType<
P extends PanelProps = PanelProps,
C extends ComponentType<P> = ComponentType<P>,
> = (
| ComponentType<P>
| WrappedComponentType<P, C>
| PanelFunctionComponentType<P, unknown>
) &
PanelStaticMetaData;
> = (ComponentType<P> | WrappedComponentType<P, C>) & PanelStaticMetaData;

export function isWrappedComponent<
P extends PanelProps,
Expand Down
4 changes: 2 additions & 2 deletions packages/golden-layout/src/LayoutManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class LayoutManager extends EventEmitter {
| ComponentConstructor
| ComponentConstructor<ReactComponentConfig>
| React.Component
| React.ForwardRefExoticComponent<any>;
| React.ComponentType;
} = { 'lm-react-component': ReactComponentHandler };

private _fallbackComponent?:
Expand Down Expand Up @@ -169,7 +169,7 @@ export class LayoutManager extends EventEmitter {
constructor:
| ComponentConstructor
| React.Component
| React.ForwardRefExoticComponent<any>
| React.ComponentType<any>
) {
if (
typeof constructor !== 'function' &&
Expand Down
1 change: 0 additions & 1 deletion packages/golden-layout/src/utils/ReactComponentHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ export default class ReactComponentHandler {
var defaultProps = {
glEventHub: this._container.layoutManager.eventHub,
glContainer: this._container,
ref: this._gotReactComponent.bind(this),
};
var props = $.extend(defaultProps, this._container._config.props);
return React.createElement(this._reactClass, props);
Expand Down

0 comments on commit 938aa07

Please sign in to comment.