swimlane / ngx-charts

:bar_chart: Declarative Charting Framework for Angular
https://swimlane.github.io/ngx-charts/
MIT License
4.29k stars 1.15k forks source link

Add types to inputs. #1274

Open NatoBoram opened 4 years ago

NatoBoram commented 4 years ago

Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'm trying to add a custom ngx-charts-legend somewhere. Problem is, I have no idea what the hell I'm supposed to write in its inputs.

For reference, here's its inputs.

@Input() data;
@Input() title;
@Input() colors;
@Input() height;
@Input() width;
@Input() activeEntries;
@Input() horizontal = false;

Here, every single property (except for horizontal) is of type any, which means we can put anything we want there. However, it is not the case, and it is quite difficult to figure out how to use the library when there's absolutely no type information available anywhere.

data seems to be of type string[], and colors seems to be of type ColorHelper, but there's no way to know more about it without hours of navigating this plate of spaghetti.

Describe the solution you'd like

A clear and concise description of what you want to happen.

I would like for all @Input() across all the library to have types.

More than that, all variables across the library should be typed.

Ideally, in tsconfig.json, noImplicitAny should be set to true.

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

One could add JSDocs comments, but TypeScript disallows specifying the type there since it's redundant.

Additional context

Add any other context or screenshots about the feature request here.

LegendEntryComponent :

@Input() color: string;
@Input() label: any;
@Input() formattedLabel: string;
@Input() isActive: boolean = false;

ChartComponent :

@Input() view;
@Input() showLegend = false;
@Input() legendOptions: any;

// remove
@Input() data;
@Input() legendData;
@Input() legendType: any;
@Input() colors: any;
@Input() activeEntries: any[];
@Input() animations: boolean = true;

ColorHelper :

scale: any;
scaleType: any;
colorDomain: any[];
domain: any;
customColors: any;
NatoBoram commented 4 years ago

It looks like ngx-charts-chart's legendOptions has a very complicated type.

import { ColorHelper } from '@swimlane/ngx-charts';

export interface NgxLegendOptions {
  title?: string;
  position?: 'below' | 'right';
  domain: string[];
  colors: ColorHelper;
  scaleType?: 'linear' | 'scaleLegend' | 'legend';
}

Seriously, having to guess all of this is time consuming! And I'm not even sure I'm right!

NatoBoram commented 4 years ago

Found some other useful types.

export enum NgxLegendPosition {
  below = 'below',
  right = 'right',
}

export enum NgxScaleType {
  linear = 'linear',
  scaleLegend = 'scaleLegend',
  legend = 'legend',
}

export interface NgxColourSet {
  name?: string;
  selectable?: boolean;
  group?: string;
  domain: string[];
}

export interface NgxLegendOptions {
  title?: string;
  position?: NgxLegendPosition;
  domain: string[];
  colors: ColorHelper;
  scaleType?: NgxScaleType;
}

In ColorHelper, the constructor is missing some types.

export class ColorHelper {
  constructor(scheme: string | ColourSet, type: ScaleType, domain: string[], customColors?) 
}
KingDarBoja commented 4 years ago

Hello @NatoBoram,

Don't worry about that, I have been submitting PR's to fix the library with the proper types. So far I have provide most of them but haven't applied that to every chart.

You can check those if you wish: #1249, #1251.

I am just waiting to get some of them reviewed and merged to continue submitting all the proper types because a single mistake can affect several charts.

Cheers!

NatoBoram commented 4 years ago

Thanks @KingDarBoja. I suggest you separate your new features and charts from the typing fixes.

I've also created #1278 with minimal changes in hope to be useful to the project since I'm using it.