Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

New: Store type parameter constraints (fixes #188) #189

Merged
merged 1 commit into from
Mar 18, 2017

Conversation

Pajn
Copy link
Contributor

@Pajn Pajn commented Mar 17, 2017

Note that Flow uses bound here instead of constraint. I decided to go with a different property because there is some differences between Flow and TS here, however I don't have anything against aligning and require the user to use other ways to distinguish Flow and TS.

@eslintbot
Copy link

LGTM

@JamesHenry
Copy link
Member

JamesHenry commented Mar 18, 2017

@Pajn would you mind elaborating on the differences? I am not too familiar with how Flow works here

@Pajn
Copy link
Contributor Author

Pajn commented Mar 18, 2017

I don't know much myself, only what I seen via the parser code.
https://github.com/Pajn/prettier/blob/fcc790a2c20e708a5af91a6ec47838b856d9d8e9/src/printer.js#L1786-L1792
The first if that checks n.bound is for Flow and the second that checks for n.constraint is this PR.

As you probably know, a type parameter constraint looks like this in TypeScript

type Result<T extends {}> = Success<T> | Failure 

while in Flow it looks like this

type Result<T: {}> = Success<T> | Failure 

See https://flowtype.org/blog/2015/03/12/Bounded-Polymorphism.html
Flow also supports specifying variance and have some special * syntax. What those really mean and how or if they would effect TS, I have no idea about unfortunately.

@JamesHenry
Copy link
Member

Fair enough! Let's stick with constraint then. I am rerunning the build

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build reran successfully and this LGTM, thanks!

@JamesHenry JamesHenry merged commit 4d755ed into eslint:master Mar 18, 2017
@Pajn Pajn deleted the type-parameter-constraint branch March 19, 2017 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants