Autodesk / react-base-table

A react table component to display large datasets with high performance and flexibility
https://autodesk.github.io/react-base-table/
MIT License
1.5k stars 165 forks source link

Type of Alignment (and other constants) in types/index.d.ts is a 'type' instead of a 'value' #298

Open saperdadsk opened 3 years ago

saperdadsk commented 3 years ago

Currently, Alignment is exported as a variable directly in index.js

export { default as Column, Alignment, FrozenDirection } from './Column';

and is defined as

export const Alignment = {
  LEFT: 'left',
  CENTER: 'center',
  RIGHT: 'right',
};

However, in types/index.d.ts, Alignment is specified as

export type Alignment = 'left' | 'right' | 'center';

which is a type, not a value. This means that a statement like

import { Alignment } from 'react-base-table'

const column = {
  align: Alignment.CENTER
}

Will fail if compiled with typescript, since types cannot be used as values:

'Alignment' only refers to a type, but is being used as a value here.ts(2693)

Later in types/index.d.ts, Column.Alignment is typed:

  export class Column<T = unknown> extends React.Component<ColumnShape<T>> {
    static readonly Alignment: {
      readonly LEFT: 'left';
      readonly CENTER: 'center';
      readonly RIGHT: 'right';
    };
    static readonly FrozenDirection: {
      readonly LEFT: 'left';
      readonly RIGHT: 'right';
      readonly DEFAULT: true;
      readonly NONE: false;
    };
  }

So the code

import { Column } from 'react-base-table'

const column = {
  align: Column.Alignment.CENTER
}

Will compile.

Alignment should likely be typed the same way, since Column.Alignment === Alignment.

Not sure exactly what the right solution is, since changing the types now is technically a breaking change to the types (Someone could currently be doing something like:

import { Column, Alignment } from 'react-base-table'

const myColumnAlignment: Alignment = Column.Alignment.CENTER

It's unclear to me if this is just happenstance, or it was actually intended.

nihgwu commented 3 years ago

it's definitely not intended and the type definition in types/index.d.ts is wrong I think, I was completely new to TS when bringing typings into this package

nihgwu commented 3 years ago

image