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

Correctly type check annotation constructors. #231

Open
tim-evans opened this issue Oct 7, 2019 · 2 comments
Open

Correctly type check annotation constructors. #231

tim-evans opened this issue Oct 7, 2019 · 2 comments
Labels
🐞 Bug This is identified as a bug or is fixing a bug
Projects

Comments

@tim-evans
Copy link
Collaborator

Currently, our type checking around annotation constructors isn't strict enough and allows invalid construction of annotations.

What do you expect to happen?

Instantiating a new annotation should be a type error when the annotation requires attributes and the attributes are omitted from the output.

@tim-evans tim-evans added the 🐞 Bug This is identified as a bug or is fixing a bug label Oct 7, 2019
@tim-evans
Copy link
Collaborator Author

tim-evans commented Oct 9, 2019

This post on the Artsy blog has a solution specifically for this:

constructor(
  attrs: {} extends Attributes
    ? { id?: string; start: number; end: number; attributes?: Attributes }
    : { id?: string; start: number; end: number; attributes: Attributes }
)

@tim-evans
Copy link
Collaborator Author

tim-evans commented Feb 13, 2020

nb. I tried doing this and it requires us to use schemas 😭

The tldr on this is that TypeScript really doesn't like it when we have different arguments to annotation constructors, especially for the AnnotationConstructor class.

@tim-evans tim-evans added this to To do in 🏭 Work Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug This is identified as a bug or is fixing a bug
Projects
🏭 Work
  
To do
Development

No branches or pull requests

1 participant