Definitelytyped: [meta] Contribution guide should document best practices for breaking changes

[thw0rted]

• [X] I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).

BLUF: I broke somebody's build by removing a long-deprecated type from a package. DT should take steps to prevent this sort of thing in the future.

Long version: For reasons too complex for this question, having the deprecated type NodeBuffer in @types/node caused deprecation warnings to show up in my code. I looked into it and there hasn't been an actual NodeBuffer class in node since at least since DT started tracking, so I put in a PR (#25500) to remove it. Because DT doesn't practice semver, this caused at least one user of @types/node to break a build where a second- or third-order dependency referenced NodeBuffer.

I have looked through the issues here and found a few tangentially related (like #7719) but I don't see any discussion of how to mark changed typings to prevent consumers from breaking on an automated update. It's my understanding that the current convention has @types/foo versioned with a number that matches the library foo it describes. The problem with this is of course that I can release 10 typings for [email protected] as I improve the description, adding more specific return types or correcting errors. Each of these typings changes (for the same version of the target library) could potentially be breaking or non-breaking, but the vocabulary we have today does not allow me to express this.

What I'd like to be able to say is "this is version 4.5.6 of the typings for node 9.X", and have npm understand that, per semver, it can update automatically to 4.6.0 of the node 9.X typings, but it should not automatically update to typings version 5.0.0 when that is created. Then, when I want to make a breaking change to the typings -- not based on changes to the underlying library, which would necessitate a major version increment in the library itself -- I could increment the typings version and users would know to check for breakage.

Is this possible? Desirable? I'm certain I'm not the first to have this kind of thought, but if it was discussed here before, I haven't found it. Regardless of the solution, this kind of problem -- how to handle breaking changes for typings within a major version of the library being described -- should get an entry in the contribution section of the README.

(Probably cc @mhegazy @RyanCavanaugh ?)

[RyanCavanaugh]

Each of these typings changes (for the same version of the target library) could potentially be breaking or non-breaking

If you change a type, any type, anywhere, in any fashion, that change is always capable of breaking downstream code. For any two definitions which are not exactly identical, there is a program that has a type error in one but not the other. This is why "just use semver" (as is often suggested) is a complete non-starter -- it's identical to just locking to an exact patch version, unless you're super interested in getting updates in JSDoc comments.

We get exactly three numbers, plus an orthogonal linear axis of "tags", to describe a library. We decided fairly early that having the typings version match the library version in the first two digits was important - major versions can obviously have breaks, and minor versions are important because new features might appear. Patch version is obviously required to rev anything, so that's where changes happen.

The alternative is to rev the major version every time someone touches the .d.ts file, then offer some out-of-band data for "Versions 421 through 837, 983 through 1023, and 1142 through 1421 map to library version 3.4", then write a separate tool that isn't NPM to manage this mapping in a user-facing way. Remember that multiple underlying major+minor versions of the package can exist simultaneously on DT, so any given "new rev" of the top-level package might correspond to any of those major+minor subversions.

Until NPM offers five-dimensional version numbers, there's nothing else we could do on our side.

Our only guidance is "Don't break people if you don't have to" which is generally naturally adhered to.

[thw0rted]

That's a great explanation and it really helps me understand why DT works the way it does. Thanks! Is the meaning of a typings version actually written out somewhere in the docs? Your explanation would really help newer users like me, and it would be great to put it out front where everybody can see it.

Also: when you say "don't break people if you don't have to", how does that impact refactoring? As an example, I saw an anonymous interface that was commonly used across Node (several classes have an address() method that returns family/host/port) so I made a named interface to simplify the type definitions. Since the interface isn't named in Node, I had to just pick a place to put it. (I couldn't find any guidance on best practices for this.) Does this guideline mean that my choice is now etched in stone, because moving the interface to a different namespace could break consuming code?

And what about deprecation, as in the PR that triggered this issue -- is it always discouraged to remove deprecated types? If there's a deprecated named type or interface that doesn't have a real-world JS counterpart, should it only be removed when the library's major version changes?

[justingrant]

@RyanCavanaugh - when you say above that "having the typings version match the library version in the first two digits was important", do you mean that every time a package revs its major or minor version, then the typings in DefinitelyTyped should also be bumped to the same version and published to NPM?

If no, then what _do_ you mean?

If yes, then how does the client side of these updates work? Is npm update smart enough to keep the versions of the library and the typings in sync? Or do I have to manually edit package.json to bump the version of @types/xxx to match the updated version of xxx in package.json?

Also, if yes then what happens if typings aren't updated at the same time a library moves to a new major or minor version?

Using a real example, the react-datepicker library has a latest of 2.0.0 but the typings file 1.1.7 was published at the same time as the 1.6.x version of the library. Was this a mistake? Should the typings versions have been incremented to match each library version?

Following the same example, the 2.0 release of the library has serious breaking changes: the entire library was refactored to use Date instead of moment.js. Over 20 props have changed their type between 1.8 and 2.0 without changing the names of those props. This makes the typings for 1.8 and the typings for 2.0+ completely incompatible. So how should this case be handled? Should the library maintainers (or me via PR!) publish a 1.8 version of the typings and a 2.0 version of the typings? Or is there some other solution needed?

[Flarna]

@justingrant see https://github.com/DefinitelyTyped/DefinitelyTyped#i-want-to-update-a-package-to-a-new-major-version Real live examples are e.g. node where you have subfolders for v6, v7,...

The first line in the typings file specifies the major.minor version of the @types package.

[justingrant]

hi @Flarna - yep I read those docs before posting, but found them both confusing and inaccurate. First, I was confused by discussion of "router v2" because I assumed that "v2" was the _new_ version of the library. Only much later did I realize they were talking about "v2" as the _old_ version. Suddenly the entire scheme made sense: old versions go into versioned subdirectories, while the main directory is for the latest version. Is that correct?

Another problem is that those docs have some content that's no longer correct because it's out of date. For example, it says:

For example, react-router depends on history@2, so react-router tsconfig.json has a path mapping to "history": [ "history/v2" ];

If you follow the link you'll see that react-router no longer depends on an old V2 version of this library. This made me wonder if TS had added better/smarter versioning support and these docs were not describing the latest/greatest way to handle versioning.

This confusion with the docs prompted my post on this issue here.

Assuming I'm confident that I understand the recommended approach, I'll try to send a PR for this section of the docs to clarify the content and fix up the outdated links.

[justingrant]

@Flarna @RyanCavanaugh - OK, PR is complete. #30746 I wrote a new section explaining how library versions relate to typings-package version. I also updated the major-version-update section to fix broken links and clarify the content.

I'm not 100% sure I got everything correct, so review needed!

[johnjesse]

I think I've followed the above.

Does this mean that if I have a package which has types already targeted at the latest version and I want to update it to use features from typescript 3+ instead of 2+ then there is no way to mark it as a major change?

Also, should it be written somewhere in the readme that moving to a new major version of typescript should be considered a major version change and so should only be done with an update of the targeted major version of the library?

I don't know what the best practices around introducing changes that rely on newer versions of typescript are, it would be good to have that clarified

[thw0rted]

It seems to me that telling everybody you can't update TS dependencies until the library happens to do a major version revision simply isn't tenable when some libraries go many years between major versions. It would make a lot more sense for DT to have a global rolling upgrade window, which is how I thought it more or less worked. DT adds support for a new version to their processing scripts after X months, then anybody is free to start using that version with the assumption that consumers have had time to move to the new compiler. Right?

[Flarna]

Using typescript 3.1 or newer features is no longer a reason for a breaking change in the definitions as typescript allows create version definitions using the field typesVersions in package.json (see e.g. typescript 3.1 release notes.

This is already using in definitions of es6-shim. I think splitting the definitions into a common part and a typescript >=3.1 and <3.1 part would be easier to maintain but according to https://github.com/DefinitelyTyped/DefinitelyTyped/pull/30477#issuecomment-439734004 this requires a new version of dtslint which should be not that far in the future.

Seems the corresponding part in readme needs also some update.

[thw0rted]

If that's going to be the guidance I'd really like to see a concrete example. It also sounds like kind of a maintenance headache, to have separate defs (or partial defs) for, say, node 8, 9, and 10, each further split by TS X, Y, and Z. Plus, this introduces the possibility of a breaking change when you update your TS version and suddenly your definitions from DT become completely different. (Just as an example, I'm thinking of cases where using mapped types on tuples makes it possible to return more specific types than would be possible otherwise, but there are plenty of other similar issues.)

[Flarna]

Well, changing typescript version includes always the risk of getting errors you don't had before because it has a better understanding of the world - which is usually a good thing.

But I fully agree that splitting definitions for major package versions and typescript version may easily result in a maintenance pain. In the end it's up to us to review carefully and avoid duplication as much as possible. And I expect it's in the end up to us to create the example...

[BendingBender]

@justingrant Love your PR. @DanielRosenwasser Could you please take a second glance at @justingrant's PR, it's been hanging in the queue since December.

[justingrant]

Yep would be great to get that either approved or denied. @DanielRosenwasser - you asked for changes in #30746 and I made requested edits within 40 minutes. Then radio silence for a few months. ;-(