Resolving dependency conflicts when importing node package typings

[mhegazy]

This issue is explained in details in #2839

Terminology

Through out this issue, i will be referring to different ways of declaring external modules, here are what i mean by them.

Ambient external module declaration

Module declarations of the form:

declare module "mod" {
    ....
}

Proper external modules

The other way of writing a declaration of a module, i.e. a declaration file with a top level import or export; e.g.:

import { Contract } from ".\core"
export var MyContract: Contract;

Script file

A file that is not a module, i.e. does not include a top level import or export. it could however, include ambient external module declarations as described above.

Background

With module resolution work done in #4154 and #4352, it is possible for node package authors to distribute typings with their packages. For these packages, users do not have to independently acquire the typings from definitely typed, but will just import the package, the the compiler will locate the typings in the package directory (either index.d.ts or by following typings property in package.json in the package directory).

As noted in #2839, for a package author interested in distributing their typings with their package, there are dependencies that they need typings for; the most common would be node.d.ts, which is possibly referenced by each package.

The way they dependency typing are authored today on definitely typed is using ambient external module declaration; this indicates that these typings exist in the global scope, and thus are susceptible to conflicts. The classic the diamond dependency issue (courtesy of @poelstra):

• myprogram
  • mylib
    • myutils@1.0
  • myotherlib
    • myutils@2.0

Now, if myUtils.d.ts uses ambient external module declaration, this is guaranteed to result in re-declaration errors.

Proposal

When resolving an import target in a node module, it is an error to include files that are not either:

1. Proper external modules, or
2. A Versioned script file - see below

For proper external modules, there are no conflicts expected, as external modules do no polute the global namespace.

For versioned files, referring to the same library, the compiler will follow a conflict resolution logic picking only the latest copy (as described by @basarat in #2839 (comment)). The files provided on the command line will be allowed to override this conflict resolution policy.

Versioned files

These are files declaring an identity, i.e. a name and a version. A versioned file is expected to contain a comment at the top of the form

/// <library name="myLibraryName" version="1.0.23.2-beta" />

Where the name field is optional, and if committed, the file name is used instead, and the version field follows the version definition in http://semver.org/

[mhegazy]

I would appreciate feedback on this issue.

Pinging a few folks who would be interested, @vladima, @RyanCavanaugh, @basarat, @poelstra, @johnnyreill, @jbrantly, @Zoltu, @weswigham and @mhfrantz.

[MicahZoltu]

In the example case, does the MyUtils JavaScript library pollute the global namespace meaning there already is a conflict at the JavaScript level? If so, how does JavaScript handle the problem?

Personally, I think as much as possible should be done to discourage global namespace pollution. Because of this, as long as there is a really good story for the non-polluting dependencies then I really don't care that much about what happens with the globally polluting story and to a certain extent, I want it to be a little painful.

For versioned files, referring to the same library, the compiler will follow a conflict resolution logic picking only the latest copy (as described by @basarat in #2839 (comment)). The files provided on the command line will be allowed to override this conflict resolution policy.

My detest for global namespace pollution goes so far as to suggest that in the case of conflict, the user should be forced to manually resolve and no auto-resolution should occur. While this is annoying, it is less likely to result in odd run time or compile time behavior that will surprise the user. This will, hopefully, also encourage authors to stop polluting the global namespace (something they really shouldn't be doing in the first place).

All of the above being said, I recognize that we are stuck compiling to a language that has a lot of existing libraries that pollute the global namespace so perhaps my stance is too harsh to be accepted by the community at large, or perhaps it simply isn't realistic to implement.

[jbrantly]

Re-pinging @johnnyreilly since the first one was messed up 😄

For versioned files, referring to the same library, the compiler will follow a conflict resolution logic picking only the latest copy

It seems to me this still has the same issue pointed out here. Namely, if myutils had some kind of breaking change between 1.0 and 2.0, it's possible that mylib is using old typings that could cause a compilation failure. (Simple example: removal or rename of an interface).

[weswigham]

IMO, from a package consumer perspective, all module declarations should be scoped to the package they are declared in, unless they are explicitly exported into the context of the package which includes it. One of my dependencies' inclusion of an older version of a library shouldn't hinder my use of a newer one, and nor should my use of a newer one interact with the older one.

Each package should be considered a unique scope whose only exports are those it explicitly chooses to export, just as with the underlying javascript. Global namespace pollution in the case of packages should be opt-in and considered very non-kosher, not "the natural way things are" using an existing feature.

External modules are already scoped correctly for this, so the issue lies with ambient external modules. I believe that ambient external modules should be scoped to the package they are declared in to help alleviate transition issues, and that the ambient external module syntax should be discouraged for use with node style packages (but not an error, as described above). I do not believe that "versioned" dts are a good idea, given that npm is already doing version and dependency management for us. We should not be redoing it, we should be working frictionlessly with it, and supporting existing patterns.

[jbrantly]

@weswigham I had the exact same thought. In fact I was writing up a proposal for it when you posted.

I made it a separate issue (#4668) so as not to detract too much from discussion of the proposal here.

[vvakame]

I think myutils should use proper external modules, it is easy way if myutils is wrote by TypeScript.
We does not have "correct" typing for myutils when myutils@1.0 and myutils@2.0 does not have API compat.
but proper external modules has it. #2338

and, I want to --disableReferenceComment options.
Now we have tsconfig.json. I think reference comment is not useful now.
(but tsconfig.json is poor, We need filesGlob or include property on it. #3232
top level module author should control all type definitions.

[poelstra]

@mhegazy I was hoping for #2839 to get some love in 1.6, so thanks :)

When resolving an import target in a node module, it is an error to include files that are not either:

1. Proper external modules, or
2. A Versioned script file - see below

Option 1 is indeed a good idea (and is what #2839 is basically about). But it still fails for deep (non-Typescript) dependency trees, which is where option 2 can be useful.

Even more 'advanced' example dependency tree:

• myprogram (native TS module)
  • mylib (native TS module, with 'proper external' typing)
    • myutils@1.0 (plain JS module)
      • subutils@3.0 (plain JS module)
    • foolib@1.0 (plain JS module)
      • subutils@4.0 (plain JS module)
  • myotherlib (native TS module, with 'proper external' typing)
    • myutils@2.0 (plain JS module)
      • subutils@4.0 (plain JS module)

(Note: I assume none of the JS modules pollute the global namespace, i.e. they are 'normal' CommonJS modules.)

In this case, because mylib is the 'first' package that knows about TS, it somehow needs to provide the typings for all non-TS stuff it exposes: myutils@1.0, foolib@1.0 but also both subutils@3.0 and subutils@4.0.

Option 2 could help with this, where it can reliably distinguish between subutils@3.0 and subutils@4.0 versions. Is this also where you intended it for, @mhegazy?

What I'm missing from this proposal, though, is how the resolution and version-lookup logic would work exactly (e.g. like how #2338 searches node_modules and uses package.json).

I'm going to type up what I think it could be doing in another post/issue.

[poelstra]

I've written a proposal for the lookup logic in a separate issue: #4673.

Key take-aways from it:

• the concept of a 'current TS package' and 'current JS package', the TS package being the one that has to provide the typings for the non-TS packages
• tried to incorporate the <library> tag (algorithm B), but also give an alternative filesystem-based solution (algorithm A)
• allow for a smooth transition between current DefinitelyTyped packages ('ambient external') to the future ('proper external')

[mhegazy]

The main problem is we do not know if a package is really polluting the global namespace or it is just authored this way because 1. it was the only way you can node typings to work before #4352, and 2. because it is convenient to write one typings file for multi-loader (aka isomorphic) packages.

In an ideal world, all typings in a node application will be in in form of a proper external module, with no global namespace pollution, and each package would export its own declaration file.

However, it is not realistic to assume that though, as 99.9% of typings today are authored as ambient external modules and not all dependencies will export their own typings, so package authors still need to re-export some .d.ts files with their package (ideally the ones already on definitely typed).

If all typings are in a proper external module format, there is no conflict resolution to handle. you can have multiple packages depending on different versions of the same library, possibly with breaking changes, and everything would work, because of the external module scoping rules.

The main problem we need to solve, is resolving the global scope conflicts. hence the version proposal; i really can not see how else to solve this.

@jbrantly i believe this is the same problem you have today. if you are using tsd, and you have a transitive dependency on two versions of the same package, you need to "flatten" them, either by picking the latest, or manually modifying the typings to create a new typings file that is compatible both versions. i would expect this to continue being the solution here as well.

One thing that i have not mentioned in this proposal, but is related, the importer, should not get errors from the package unless they have a way of witnessing it. i.e. in the example in the original post, if there is a missing/renamed declaration, in myUtils@4.0 (breaking change from myUtils@3.0) that breaks the dependentmyLib, the importingmyProgram` should not see an error unless they use a type depending on this missing declaration. The typechecker is set to do this today, it is just that we do not use it, but should be straight forward to do.

@poelstra, your proposal in #4673 is for locating a typings file, which is a fine proposal, but does not handle conflicts in the global namespace. if your utils@3.0 and utils@4.0 both define variable or modules with the same name in the global scope, there will be errors.

@Zoltu, and @weswigham we can not just say no global name space pollution; though I do agree it is a bad practice. i think the best we can do is provide guidance.