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

Migrate TS typings to DefinitelyTyped #1778

Closed
quantizor opened this issue Jun 1, 2018 · 21 comments
Closed

Migrate TS typings to DefinitelyTyped #1778

quantizor opened this issue Jun 1, 2018 · 21 comments
Assignees

Comments

@quantizor
Copy link
Contributor

quantizor commented Jun 1, 2018

For 4.0 and beyond, we've decided to move styled-components typings out of this repo to https://github.com/DefinitelyTyped/DefinitelyTyped. This aligns with our strategy on Flow: to allow the community to type the library at their own pace elsewhere.

copied from below for visibility:

The reality is none of the core team makes extensive use of the TS ecosystem. Thankfully, there are a few dedicated members of the s-c community that are willing to pitch in to handle typing-related issues, @Igorbek for example, but this is all volunteer time and he may not always be available.

We receive a massive amount of issues, PRs, and such on TS typings weekly. So many that constantly triaging them hinders our ability to focus on actually improving the library. Moving the TS typings to a community location ends up being the most pragmatic decision to ensure that our time is not being scuttled on things for which we don't have enough context.

I realize this may not be a popular choice, but it's the right one for styled-components and will allow you, members of the TS community, to upgrade and use the latest functionality without waiting on us. If you are concerned about the community typings falling behind forthcoming library changes, we will have a beta period and thoroughly communicate things ahead of time so people can prepare.

Anyone who's interested in doing this is welcome to start immediately. We will still accept updates to the typings that will fit in a minor point release prior to 4.0.

@quantizor
Copy link
Contributor Author

cc @styled-components/typers

@Igorbek
Copy link
Contributor

Igorbek commented Jun 1, 2018

I will move it over.

@quantizor
Copy link
Contributor Author

Awesome, thank you @Igorbek 🙇

@hatashiro
Copy link

hatashiro commented Jun 3, 2018

In the TypeScript community, having type definition in the main repository is accepted as a good practice. There are plenty of reasons, if listing some:

  • Having type definition in a repository makes it much easier to sync versions between code and types.
  • DefinitelyTyped is a huge monorepo and somewhat difficult to access. Also, because it's not owned by styled-components, we lose flexibility to change how we maintain it. For example, DefinitelyTyped enforces a specific way to provide tests, etc.
  • In DefinitelyTyped, it's much more difficult to track issues and review pull requests. There are 2000+ open issues and 100+ open pull requests. There is no way to track a specific directory or keyword in GitHub, so contributors are likely to stop tracking them.
  • Having type definitions in styled-components makes contributors willing to contribute more, because it makes them feel like contributing to styled-components itself, instead of DefinitelyTyped.
  • Having type definition in a repository is also helpful when using plain JavaScript. Tools like VS Code provides autocompletion and type information even with plain JS when there is typing in a package.

I can't actually come up with a good reason to separating the definition but for maintenance reason. If maintenance is an issue, I (and other typers) are quite familiar with TypeScript and can always help you with it from now.

In conclusion, can we please revise this?

@quantizor
Copy link
Contributor Author

So first, let me say that this decision wasn't made lightly. It's been discussed off and on over several months between @mxstbr, @kitten, and I.

The reality is none of the core team makes extensive use of the TS ecosystem. Thankfully, there are a few dedicated members of the s-c community that are willing to pitch in to handle typing-related issues, @Igorbek for example, but this is all volunteer time and he may not always be available.

We receive a massive amount of issues, PRs, and such on TS typings weekly. So many that constantly triaging them hinders our ability to focus on actually improving the library. Moving the TS typings to a community location ends up being the most pragmatic decision to ensure that our time is not being scuttled on things for which we don't have enough context.

I realize this may not be a popular choice, but it's the right one for styled-components and will allow you, members of the TS community, to upgrade and use the latest functionality without waiting on us. If you are concerned about the community typings falling behind forthcoming library changes, we will have a beta period and thoroughly communicate things ahead of time so people can prepare.

Thank you for your understanding.

@hatashiro
Copy link

I really appreciate your detailed comment. I understand that handling TS issues and pull requests in the same repo becomes a burden for maintainers.

I think if it's the reason, we can try a different approach? For example, we can separate types to its own repository, let's say styled-components/types, and download the type definition file only on npm publish of styled-component. We can just use curl for the download, don't need a git submodule. I think it is just another possible solution, it isn't a very common practice though.

I understand that the final decision is up to the core team and I will completely respect the decision.

@quantizor
Copy link
Contributor Author

let's say styled-components/types, and download the type definition file only on npm publish of styled-component

The problem with this strategy is there is no semver protection for the types. It would be super easy to accidentally release a breaking type change with an otherwise minor library release.

Also, part of the reasoning for DefinitelyTyped is they own the @types scope which seems to be the de-facto place to look for types in the TS ecosystem. It just makes sense to put them there and expands the number of people that could potentially chip in to maintain them exponentially.

@quantizor quantizor mentioned this issue Jun 22, 2018
19 tasks
@DanielRosenwasser
Copy link

I know it's been a month, but I was just linked to this and I want to elaborate on our position.

As a member of the TypeScript team, I think this was the right move. Types that are not generated directly from .ts files should typically be put on DefinitelyTyped.

Thanks @Igorbek and others in @styled-components/typers who've helped out with the types in this project (even though I don't use it myself - I'm not a front-end dev 😉).

The reality is that pinning the responsibility of .d.ts maintenance on library authors puts too much burden on them, whereas DefinitelyTyped provides an opportunity to

  1. get some level of oversight from the TypeScript team
  2. get more wide review from peers
  3. easily use testing infrastructure like dtslint
  4. easily test against other packages on DefinitelyTyped that reference this package
  5. decouple versioning of the package with the .d.ts files, allowing .d.ts fixes to be shipped without a package publish.

So I would say this was a reasonable move.


To address some points:

Having type definition in a repository makes it much easier to sync versions between code and types.

This really depends on how you communicate with the community and how much patience a maintainer has.

DefinitelyTyped is a huge monorepo and somewhat difficult to access. Also, because it's not owned by styled-components, we lose flexibility to change how we maintain it. For example, DefinitelyTyped enforces a specific way to provide tests, etc.

The monorepo is problematic for new contributors, but if you have ideas for better testing strategies, we're interested.

In DefinitelyTyped, it's much more difficult to track issues and review pull requests. There are 2000+ open issues and 100+ open pull requests. There is no way to track a specific directory or keyword in GitHub, so contributors are likely to stop tracking them.

We're not sure how to manage issues, but pull requests are above 100 because of the influx. Right now they're at 50, not all of which are actually actionable.

Having type definition in a repository is also helpful when using plain JavaScript. Tools like VS Code provides autocompletion and type information even with plain JS when there is typing in a package.

VS and VS Code both provide what's called "automatic type acquisition", so JS users get the types either way.

@hatashiro
Copy link

@DanielRosenwasser Thanks for your comment. I didn't know about VS Code type acquisition. It's great.

It may be just my personal experience, but I found that types in DefinitelyTyped are more likely to be out of sync, than ones installed with JS packages. As you mentioned, I guess, new contributors are more willing to contribute when it's in the main repo.

However, I understand maintaining type definitions is a burden and moving it to DefinitelyTyped reduces the burden. I respect the decision of maintainers and believe it's the right move.

@hendry1412
Copy link

So the types won't be fixed until they are moved to a separate repository? When will that take place? Does it have to wait for the 4.0 release?

@quantizor
Copy link
Contributor Author

No, they can be moved at any time. Someone just has to do it.

@AllainPL
Copy link

AllainPL commented Aug 6, 2018

As we are using s-c with TS at our company I feel I could help out, but it would be my kinda first bigger contribution so I want to make sure what is to be done ;)

  1. PR to https://github.com/DefinitelyTyped/DefinitelyTyped/ - with types from styled-copmponents in proper DefinitelyTyped structure.
  2. PR to s-c removing the types
  3. PR to styled-components-website - I'm deducting this from your recent tweet

Should i be aware of any other problems with this?
I was thinking about it and I'm concerned about two things:

  1. Coordinating the release cycle of s-c and DefinitelyTyped - to make sure types won't get missing in any release.
  2. Dependencies - e.g. styled-system and their typings maybe?

@quantizor
Copy link
Contributor Author

That looks right to me @AllainPL!

Coordinating the release cycle of s-c and DefinitelyTyped - to make sure types won't get missing in any release.

For major releases, there will be a beta period to accommodate this sort of thing. Is making an issue against DT with the relevant API changes the right way to go or to make an issue here and tag people to update flowtyped and DT?

Dependencies - e.g. styled-system and their typings maybe?

IDK, I think that's up to the individual lib authors

@Igorbek
Copy link
Contributor

Igorbek commented Aug 7, 2018

I actually thought that it'd be started from 4.0.

@quantizor
Copy link
Contributor Author

I actually thought that it'd be started from 4.0.

Yeah but I don't think there will be another minor release before 4.0 at this point. The global style thing is taking too long.

@AllainPL
Copy link

@probablyup looks like you did the heavy lifting ;) - I was too slow ;(

I checked the version of s-c from #1922 with emulated types/styled-components package against my project and everything looks fine.

@quantizor
Copy link
Contributor Author

@AllainPL thanks much for testing it out!

@quantizor
Copy link
Contributor Author

The migration is complete. Please file any future TypeScript related issues there.

Thanks all.

@oriSomething
Copy link

@utatti I think if the project isn't written in TS, it actually a good thing that the types move to DefinitelyTyped (The TS community).
Just look at immutable.JS that has outdated types and the maintainers refuse to fix types of old version (which is still the latest!) so it won't publish new versions of the lib. If it was by the community, both Flow and TS users would enjoy much better types. Not mention you need to manually remove a line in the definitions of Flow to make it work with a new versions of Flow

@simonbuchan
Copy link
Contributor

I've got some v4 typings nearly done, but for some reason the tests are failing as the styling callback props are getting typed as any. Could anyone take a look at my changes and see if they can see whats up?

DefinitelyTyped/DefinitelyTyped@ba75fbf

@quantizor
Copy link
Contributor Author

@Igorbek could you help out? ^

@mxstbr mxstbr mentioned this issue Dec 19, 2018
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants