Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing [key: string]: any from interfaces, and use type generics in function definitions. #3416

Closed
timbuckley opened this issue Jan 9, 2020 · 1 comment · Fixed by #3566 · May be fixed by #4177
Closed

Consider removing [key: string]: any from interfaces, and use type generics in function definitions. #3416

timbuckley opened this issue Jan 9, 2020 · 1 comment · Fixed by #3566 · May be fixed by #4177

Comments

@timbuckley
Copy link
Collaborator

timbuckley commented Jan 9, 2020

Do you want to request a feature or report a bug?

Feature (for TypeScript users of Slate).

Problem discussion

In the current version of Slate core (v0.57.1), interfaces are declared in the following way

interface Text {
  text: string;
  [key: string]: any;
}

...and most Slate functions are written in a form like the following:

const Text = {
  //...
  decorations(node: Text, decorations: Range[]): Text[] {/* ... */}
}

The problem with this is that TypeScript users would likely want to constrain their Text nodes in their own way, above and beyond the underlying Text, Editor, etc interfaces, while still allowing them to be used directly in helper functions without using as type coercions.

For example:

interface CompanyText extends Text {
  isBold: boolean
  fontSize: number
}

const myTextNode: CompanyText = {text: 'foo', isBold: false, fontSize: 13}

Text.decorations(myTextNode, [])
// Type error! `myTextNode` is not of type `Text`

The CompanyText type is still overly permissive because a typo like isBeld is still allowed, despite the user's extended interface (thanks to key-any). And the user would also have to use myTextNode as Text when calling Text.decorations() on it, otherwise TypeScript will complain.

Proposed solution

Use type generics in functions, and remove the [key: string]: any from interfaces

interface Text {
  text: string;
}

const Text = {
  // ...
  decorations<T extends Text>(node: T, decorations: Range[]): T[]   {/* ... */}
}

This will allow TypeScript consumers of Slate to use it in the CompanyText example above.

@timbuckley timbuckley changed the title Consider removing [key: string]: any from interfaces and use type generics Consider removing [key: string]: any from interfaces, and use type generics in function definitions. Jan 9, 2020
@miracle2k
Copy link

This would also be a huge help in migrating existing code over to the new API, as I am currently undertaking. For example, an old-style call such as editor.setNodeByKey does not show a typing error once I switch the type over the new new Editor (which has an index-signature).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment