Fix type/docs for VivView(er) and Consumers.

[ilan-gold]

Fixes #406

Background

In #397 we introduced types via jsdoc but failed to correctly type the VivViewer.

Change List

• Move the docstring from the class declaration to the constructor

Checklist

• Update JSdoc types if there is any API change.
• Make sure Avivator works as expected with your change.

[ilan-gold]

@andreasg123 Can you give this a whirl? The .ts looks more correct now and // @ts-check doesn't seem to give any bad errors for any of the React components anymore. Even if this doesn't work, this is definitely an improvement in the types so I'd merge this anyway.

export default VivViewer;
export type ViewStateChange = (event: Object) => any;
export type Hover = (info: Object, event: Object) => any;
export type HandleValue = (valueArray: Array<number>) => any;
export type HandleCoordinate = (coordnate: Object) => any;
export type HoverHooks = {
    handleValue: HandleValue;
    handleCoordinate: HandleCoordinate;
};
export type LayerProps = {
    /**
     * Props for the layers in each view.
     */
    layerProps: any[];
    /**
     * Whether or not to randomize which view goes first (for dynamic rendering of multiple linked views).
     */
    randomize?: boolean | undefined;
    /**
     * Various `VivView`s to render.
     */
    views: Array<import('../views').VivView>;
    /**
     * List of objects like [{ target: [x, y, 0], zoom: -zoom, id: 'left' }, { target: [x, y, 0], zoom: -zoom, id: 'right' }]
     */
    viewStates: Array<object>;
    /**
     * Callback that returns the deck.gl view state (https://deck.gl/docs/api-reference/core/deck#onviewstatechange).
     */
    onViewStateChange?: ViewStateChange | undefined;
    /**
     * Callback that returns the picking info and the event (https://deck.gl/docs/api-reference/core/layer#onhover
     * https://deck.gl/docs/developer-guide/interactivity#the-picking-info-object)
     */
    onHover?: Hover | undefined;
    /**
     * Object including utility hooks - an object with key handleValue like { handleValue: (valueArray) => {}, handleCoordinate: (coordinate) => {} } where valueArray
     * has the pixel values for the image under the hover location and coordinate is the coordinate in the image from which the values are picked.
     */
    hoverHooks?: HoverHooks | undefined;
};
/**
 * @callback ViewStateChange
 * @param {Object} event
 * @ignore
 */
/**
 * @callback Hover
 * @param {Object} info
 * @param {Object} event
 * @ignore
 */
/**
 * @callback HandleValue
 * @param {Array.<number>} valueArray pixel values for the image under the hover location
 * @ignore
 */
/**
 * @callback HandleCoordinate
 * @param {Object} coordnate The coordinate in the image from which the values are picked.
 * @ignore
 */
/**
 * @typedef HoverHooks
 * @type {object}
 * @property {HandleValue} handleValue
 * @property {HandleCoordinate} handleCoordinate
 * @ignore
 */
/**
 * @typedef LayerProps
 * @type {object}
 * @property {Array} layerProps  Props for the layers in each view.
 * @property {boolean} [randomize] Whether or not to randomize which view goes first (for dynamic rendering of multiple linked views).
 * @property {Array.<import('../views').VivView>} views Various `VivView`s to render.
 * @property {Array.<object>} viewStates List of objects like [{ target: [x, y, 0], zoom: -zoom, id: 'left' }, { target: [x, y, 0], zoom: -zoom, id: 'right' }]
 * @property {ViewStateChange} [onViewStateChange] Callback that returns the deck.gl view state (https://deck.gl/docs/api-reference/core/deck#onviewstatechange).
 * @property {Hover} [onHover] Callback that returns the picking info and the event (https://deck.gl/docs/api-reference/core/layer#onhover
 *     https://deck.gl/docs/developer-guide/interactivity#the-picking-info-object)
 * @property {HoverHooks} [hoverHooks] Object including utility hooks - an object with key handleValue like { handleValue: (valueArray) => {}, handleCoordinate: (coordinate) => {} } where valueArray
 * has the pixel values for the image under the hover location and coordinate is the coordinate in the image from which the values are picked.
 */
/**
 * @type {{ new(props: LayerProps): { props: LayerProps, state: any } }}
 */
declare const VivViewer: new (props: LayerProps) => {
    props: LayerProps;
    state: any;
};

[andreasg123]

@ilan-gold, I'm now getting this error:

Type error: 'VivViewer' cannot be used as a JSX component.
  Its instance type '{ props: LayerProps; state: any; }' is not a valid JSX element.
    Type '{ props: LayerProps; state: any; }' is missing the following properties from type 'ElementClass': render, context, setState, forceUpdate, refs

  136 |   }
  137 |   return (
> 138 |     <VivViewer
      |      ^
  139 |       layerProps={layerProps}
  140 |       views={views}
  141 |       viewStates={viewStates}

[andreasg123]

Apparently, inheritance can be described like this: microsoft/TypeScript#20077
The parent class should be PureComponent<LayerProps> if JSDoc supports generics.

Definition for PureComponent: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/v16/index.d.ts#L508

[ilan-gold]

@andreasg123 I couldn't get the generic or the extension working, but I did try wrapping it in a functional component and then documenting that. Seems to work for me in Avivator. Let me know!

[andreasg123]

This mostly works. I have to figure out how to produce the correct ViewStateChange type:

Type error: Type 'ViewStateChange | undefined' is not assignable to type 'import("/Users/andreas.girgensohn/go/src/github.com/10XDev/ziggy/client/node_modules/@hms-dbmi/viv/dist/viewers/VivViewer").ViewStateChange | undefined'.
  Type 'ViewStateChange' is not assignable to type 'import("/Users/andreas.girgensohn/go/src/github.com/10XDev/ziggy/client/node_modules/@hms-dbmi/viv/dist/viewers/VivViewer").ViewStateChange'.
    Types of parameters '__0' and 'event' are incompatible.
      The 'Object' type is assignable to very few other types. Did you mean to use the 'any' type instead?
        Type 'Object' is missing the following properties from type '{ viewId: any; viewState: any; oldViewState: any; }': viewId, viewState, oldViewState

  141 |       viewStates={viewStates}
  142 |       hoverHooks={hoverHooks}
> 143 |       onViewStateChange={onViewStateChange}
      |       ^
  144 |     />
  145 |   );
  146 | };

I'm using this definition:

type ViewStateChange = ({
  viewId,
  viewState,
  oldViewState,
}: {
  viewId: any;
  viewState: any;
  oldViewState: any;
}) => void;

[andreasg123]

I think that I'm good to go. I fixed the previous error like this. That didn't work.

onViewStateChange={(onViewStateChange as unknown) as VivViewer.ViewStateChange}

This worked:

type ViewStateChange = (event: Object) => void;

It shouldn't be difficult to fix this error in my project:

Type error: Type '{ layerProps: { loader: ZarrPixelSource<["t", "c", "z"] | ["c", "t", "z"] | ["z", "t", "c"] | ["t", "z", "c"] | ["z", "c", "t"] | ["c", "z", "t"]>[] | TiffPixelSource<[...] | ... 4 more ... | [...]>[] | undefined; ... 15 more ...; lensBorderRadius: number; }[]; views: any[]; viewStates: any[]; hoverHooks: any; onVie...' is missing the following properties from type '{ layerProps: any[]; randomize: boolean | undefined; views: VivView[]; viewStates: object[]; onViewStateChange: ViewStateChange | undefined; onHover: Hover | undefined; hoverHooks: HoverHooks | undefined; }': randomize, onHover

  136 |   }
  137 |   return (
> 138 |     <VivViewer
      |      ^
  139 |       layerProps={layerProps}
  140 |       views={views}
  141 |       viewStates={viewStates}

Thank you very much for the quick response.

[andreasg123]

Could you please add this to ImageLayer.js (copied from MultiscaleImageLayer.js)?

 * @param {String} props.id Unique identifier for this layer.

The following better describes VivView and allows for easier subclassing. This JSDoc documentation may be helpful for describing argument types: https://jsdoc.app/tags-type.html

diff --git a/src/views/VivView.js b/src/views/VivView.js
index 1245930..6dda388 100644
--- a/src/views/VivView.js
+++ b/src/views/VivView.js
@@ -40,7 +40,7 @@ export default class VivView {
    * @param {object} [args.viewState] incoming ViewState object from deck.gl update.
    * @param {object} [args.oldViewState] old ViewState object from deck.gl.
    * @param {object} [args.currentViewState] current ViewState object in react state.
-   * @returns {object} ViewState for this class (or null by default if the ids do not match).
+   * @returns {?object} ViewState for this class (or null by default if the ids do not match).
    */
   filterViewState({ viewState }) {
     const { id, height, width } = this;
@@ -50,8 +50,8 @@ export default class VivView {
   /**
    * Create a layer for this instance.
    * @param {Object} args
-   * @param {ViewState} args.viewStates ViewStates for all current views.
-   * @param {number} args.props Props for this instance.
+   * @param {Object<string,Object>} args.viewStates ViewStates for all current views.
+   * @param {Object} args.props Props for this instance.
    * @returns {Layer} Instance of a layer.
    */
   // eslint-disable-next-line class-methods-use-this,no-unused-vars

Otherwise, your changes work very well for me without any TypeScript errors. Thanks again.

[ilan-gold]

@andreasg123 Just pushed some changes if you want to give them a whirl. Thanks again!

[andreasg123]

@ilan-gold, it works now without problems for me. However, you introduced an inconsistency. For MultiscaleImageLayer and XRLayer, you have id defined twice for the layer parameters, the first time as optional @property {String=} id and the second time as required @property {String} id.