-
Notifications
You must be signed in to change notification settings - Fork 80
feat: add types lookup #103
Conversation
…ype files for the package and add that information to the package if so
update coverage floor
README.md
Outdated
### `types` and `flow` fields | ||
|
||
If you do not have a `types` or `flow` field, then it will check if | ||
corresponding `*.d.ts` or `*.flow.js` files exist for your package | ||
entry file and add them to the `package.json`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like we shouldn't be first-class reifying two specific implementations of type systems - both may not be around forever, and additional options may become popular. Could we come up with a single generic field that has a "flavor" property that can point to TS, Flow, or something in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This model was landed on in the rfc process and nobody had an issue with it then. I don't think prematurely worrying about potential future implementations is worth the worry today. They'd all eventually have to denote what "kind" they are, and having that be their attribute name is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware that the RFC process settled on doing it this way for flow - I'd prefer further discussion on the RFC call.
Note that I'm also pointing out that Flow may be largely unused quite soon, which would leave this as a permanent wart - as opposed to simply an unused "flavor" value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/microsoft/TypeScript/blob/main/package.json#L23
Looks like common practice is the typings
attribute, not types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types
is preferred, TypeScript is just old and no-one bothered to migrate to use 'types' (because both work) (typings used to be the name of the thing which turned into DefinitelyTyped.)
The "flow"
attribute isn't used by flow, it was added as a request during the RFC meetings to make it not just 'support TypeScript' - the flow team figured this could be something they use in the future but it's also useful at an ecosystem level for asking questions like 'how many packages have flow types' / 'does this package ship flow types'.
I don't think anyone would argue too strongly in favor of keeping it if you wanted to remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a commit that removes the flow
stuff. Adding it in the future if it turns out we want it is easy enough, and doesn't alter what we're implementing here. Best to wait till we need it than to try to prematurely implement something that's not needed or used.
It appears that either conventional practices shifted since the original PR, or these attributes really should live in multiple places. Pinging @orta who can hopefully weigh in? The question is "Should this still work as recommended, specifically with these two attributes? What is the difference between these and the |
Getting these to 100 is a future task
no more flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, makes sense to me (And would close out my RFC 👍🏻 )
This PR will fill the
types
andflow
field when TS/Flow would have inferred support for a package based on their corresponding file system structure.References
Closes #92