elastic-charts icon indicating copy to clipboard operation
elastic-charts copied to clipboard

Improve exported type interfaces

Open nickofthyme opened this issue 3 years ago • 1 comments

When developing types in charts src/ we tend to create types that are to be used internally and modify these types in the component API. For example

export interface LineAnnotationSpec {
  style?: RecursivePartial<LineAnnotationStyle>;
  // rest...
}

Chart props are commonly defined prior to defining the chart spec component which requires users to pull the prop types from the Spec component itself to get the modified type. Notice below, instead of being able to easily import and assign LineAnnotationStyle you have to use the modified type as LineAnnotationProps['style'].

const myStyles: LineAnnotationStyle = {}; // throws error

const myStyles: LineAnnotationProps['style'] = {} // no error

We have export modified spec types under <SpecType>SpecProps types according to defaults and overrides. However, this is not the case for all other types and these types are used more often to access nested prop types.

Solution

I propose that we create a mechanism and or naming convention to refer to export types based on whether they are internal or external. This would ideally be simple and not cause too much noise in the repo. Would have to discuss a good solution with the team.

Related #1007

nickofthyme avatar May 06 '22 17:05 nickofthyme

Yep, you are right, there should be two main types: one just for the API side and one used internally. In the example provided, we could have something along those lines:

  • API type LineAnnotationStyleAPI = RecursivePartial<LineAnnotationStyle>
  • Internal type LineAnnotationStyle

Should we flatten the external/API types avoiding reusing modifiers like RecursivePartial,Omit, Pick etc? like:

type LineAnnotationStyleAPI = {
line?: {
  stroke?: Color, 
  strokeWidth?: number,
  opacity?: number,  
  dash?: number[];
},
details?:{
  fontSize?: number;
  fontFamily?: string;
  fontStyle?: FontStyle;
  fill?: Color;
  padding?: number | SimplePadding;
}
}

markov00 avatar May 11 '22 08:05 markov00