microsoft / dtslint

A utility built on TSLint for linting TypeScript declaration (.d.ts) files.
MIT License
925 stars 86 forks source link

unpublished versions in definitelytyped-header-parser cause installation failure #235

Open aoberoi opened 5 years ago

aoberoi commented 5 years ago

When definitelytyped-header-parser updates to include new versions of the typescript package, and those versions are not published to npm, it causes dtslint not to work.

For example, at the time of this writing, there is no version 3.5 or 3.6 published. However, when dtslint uses TypeScriptVersion.all to seed the generation of package.json files in the installer, those version numbers are used for the typescript dependency, and then npm install ... is run in those directories. This causes dtslint to fail with the following error:

npm ERR! notarget No matching version found for typescript@3.5
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

This is an important issue for maintainers who use dtslint to verify their type definition files. I happen to maintain @slack/web-api, which depends on dtslint for this reason. We hit this issue here: https://travis-ci.org/slackapi/node-slack-sdk/jobs/538163092.

Perhaps if definitelytyped-header-parser is used in other places that depend on this behavior (so it cannot be changed) then dtslint can catch these types of errors and safely ignore them. Or perhaps TypeScriptVersion.all can be filtered for just the versions that are published to npm before installations are attempted.

aoberoi commented 5 years ago

It seems like there was an attempt to fix the exact same issue in https://github.com/microsoft/dtslint/commit/d843ba11c4205f451493f7968e107a8fb62bbfa2, which rolls back the definitelytyped-header-parser dependency to exclude 3.6. however, it would need to be rolled back to 1.0.1 to exclude 3.5 as well, for this issue to be resolved.

aoberoi commented 5 years ago

Well, now that TS 3.5 is out, this is no longer breaking our builds. However, I think we need a system in place to prevent this issue again.

Do the maintainers of this package plan to always ship with a pinned version of the definitelytyped-header-parser dependency (as opposed to a semver range)?

This seems tedious because it means each time someone ships that dependency, someone should also consider whether this package should update, but it should work. One reason it might not work is if there’s an update in that dependency that isn’t related to a new version, but it happens to occur after an unreleased version of TS is already referenced.

What are the use cases for adding unreleased versions of TS to definitelytyped-header-parser?

sandersn commented 5 years ago

The versions in definitelytyped-header-parser are updated to include the upcoming version of typescript in order to support Automatic Type Acquisition for JS authors who are using typescript@next. We follow these steps just after a release: https://github.com/microsoft/TypeScript/wiki/Release-Activities#types-publisher-and-definitelytyped-header-parser. As you can see, definitelytyped-header-parser is a tightly-coupled dependency of dtslint and types-publisher. I changed the versioning scheme to make it clear that it matches typescript's versions and not semver.

If dtslint doesn't properly install typescript@next instead of typescript@3.7 (at the time I write this), it's a bug, I think. 93168cc92bf9af6cf9373848fea2a2ab2c9be7a4, from December 2018, was supposed to prevent this bug, in fact, by converting the latest version to next. Do you observe this bug with 3.7 now?