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.

[mhfrantz]

Since TS has structural typing, couldn't you nest the declarations from the dependent modules within the namespace of the module that uses them, and still have the interoperability that you would want for any exported portion of the interface?

For example, if myLib uses myUtil, which in turn declares the Foo class, then it could be declared as myLib.myUtil.Foo. If myOtherLib uses a different version of myUtil which declares the Foo class, then it would be myOtherLib.myUtil.Foo. Any attempt to interoperate between myLib and myOtherLib via an exchange of Foo instances would work if they had the same structure. If they did not have the same structure, then that incompatibility stems from a structural incompatibility, and from not some artifact of the TS type system.

The downside would be that you would define some modules repeatedly within each context they are used. That would increase the computational resource requirements for the compiler.

[jbrantly]

@mhegazy I guess my point about versioning and picking only the latest copy is that it seems like only a halfway solution instead of a full solution. It fixes one problem (duplicate declarations) but leaves another (incompatible declarations). It would be ideal if both problems could be solved (which I believe is possible).

[weswigham]

@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.

I feel it is reasonable to scope the present ambient external module syntax to packages and then provide an explicit escape syntax to pollute the global environment. I think this is the easiest to transition to better practices with and the easiest to consume and have consumed packages "just work".

[heycalmdown]

If I have no version conflict with DefinitelyTyped typings, would it not be 'definition duplicate'? Like below dependencies tree:

• myprogram (native TS module)
  • mylib (native TS module, with 'proper external' typing)
    • node/node.d.ts (plain JS module from DefinitelyTyped)
  • myotherlib (native TS module, with 'proper external' typing)
    • node/node.d.ts (plain JS module from DefinitelyTyped)

Then, I can use it from now after a fashion.

[poelstra]

@mhfrantz

Since TS has structural typing, couldn't you nest the declarations from the dependent modules within the namespace of the module that uses them

I believe this is already exactly what happens, when 'proper external' typings are used.

[poelstra]

@weswigham

and then provide an explicit escape syntax to pollute the global environment

This is a great idea, it would also allow 'proper external' modules to indicate that they do make stuff globally available (e.g. in case of a shimming library).

[poelstra]

@mhegazy

@poelstra 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.

Of course, but the way Node modules work, you'd have to 'try hard' to pollute the global scope (e.g. assign to a property on global), and in contrast to what most DT typings seem to indicate today, most packages indeed don't do global stuff when loaded as CommonJS.

So it makes sense to (like @weswigham proposed) have to more explicitly indicate globals in the typing (e.g. global myVar; or export global myVar;). The latter isn't currently possible to do using 'proper external' typings, although it is possible in the actual JS. So it'd be a nice addition anyway, I suppose.

This is not really a breaking change, btw, because people only have to add that explicit global syntax when both:

• the package really pollutes global scope in CommonJS mode (very unlikely), and
• the typing is included via the 'proper external' mechanism or my 'legacy mode' proposal (which aren't possible with current stable versions yet).

With the proposal I made (#4673), you have the benefit that you can keep using 'isomorphic typings' (i.e. with declare module "..." { }, for use in browser, node, etc,), because of the 'legacy mode' I described. Any 'accidentally' global stuff in it will become local in the TS typing (just as it becomes local in the JS part too, when loaded as CommonJS). Unless one explicitly marks it as global.

And if someone explicitly marks something as global, and you have another package that marks the same variable/type as global (which could be two different packages, not just utils@3 and utils@4), then as @jbrantly says, you'll have a problem anyway, as it will likely be broken at runtime too.

I'm not sure that last one is something that can really be fixed. For example, what would happen when loading two incompatible Promise libraries, which both try to install themselves as global? Which one is actually going to 'win' at runtime, so which part of the global exports of the typings should 'win'? (Although the 'local' types probably still function perfectly.)

The case with node.d.ts itself is a whole other story to me: that thing feels more like a lib.d.ts. I.e. in the browser, you have lib-browser.d.ts, in node you have lib-node.d.ts.
In contrast to what's done today, i.e. every package referencing their own copy of node.d.ts, I believe a package should not reference node.d.ts at all, and the 'end user' should provide it. After all, it's the end user who decides to run it on e.g. node 0.10 or 0.12, and if the package relies on e.g. a Promise object to be available, it should indeed fail when I feed it a 0.10 node typing (without also feeding it a promise library that globally exports a Promise).

[jbrantly]

I wanted to point out some additional things after thinking on this today.

1. There's been mention about global scoping but I wanted to give a concrete example of the issue. Using the dependency tree in the OP, this proposal fixes the "duplicate" declarations issue. However, it still leaks the "myutils" module globally. So if I were to put import myUtils from 'myUtils' in myprogram.ts, TS will happily compile without complaint, but I would have issues at runtime since myutils is not actually a declared dependency of my application in JS-land.

2. There is a lot of discussion around the concept of "proper external modules". My understanding of the concept is that if all modules in the dependency tree used proper external modules then all of these issues would simply go away because everything would be scoped locally. More specifically, the dependencies of any given module would be scoped locally (myutils@1.0 definition would only be scoped to mylib, and therefore there is no chance of collision with myutils@2.0). However, this approach has a few problems that I can identify:

   1. If any dependency in the chain does not use a proper external module, then all bets are off.
   2. None of the DT definitions use it, exacerbating the previous point
   3. It is not friendly to isomorphic definitions without duplicating definitions (or risking conflicts, again)
   4. It does not allow module merging. For instance, what if I require some module that adds to the interface of another module. I realize this is probably not a common or suggested thing but it does happen (jquery comes to mind).

   The "legacy mode" in Proposal for correct version lookup in deeply embedded non-TS external modules #4673 mostly addresses the first two points, but still allows global leaks. I really don't think there is anything inherently wrong with declare module "name" except that it creates the module globally. Global leaking is the main source of the issues presented: duplication and conflict.

At risk of seeming overbearing I would really like to point people to #4668 which I believe provides a mechanism that solves all of these issues as it directly addresses the root issue of a package's dependencies being made global.

[jbrantly]

Regarding the global keyword, ie global myVar, I feel that is the wrong way to go about it. Normal definition files today can already declare globals. Assuming #4668 was in place, any definition file could declare globals just like they do today but with much less chance of conflict with another global. For example, assuming a promise library that adds a global Promise.

• myprogram
  • mylib
    • promiselib@1.0 // using Proposal: Localized typings for definition files #4668, the Promise global here would not be leaked to myprogram
  • myotherlib
    • promiselib@2.0 // again, not leaked, and therefore no conflict

Note that Promise would not be available to myprogram in this case. If it did need to be made available, then the program author can do so:

• myprogram
  • mylib ...
  • myotherlib ...
  • promise@3.0 // The Promise global here would be available to myprogram

[weswigham]

The fact that normal definition files today declare globals as the
default is part if the problem. If we follow es6 semantics for modules,
each DTS should have its own module scope, globally polluting only things
explicitly attached to the global object. Meaning the default to be
module/package scoping, with opt-in global scoping. IMO, anything currently
considered "global" should be packaged scoped, with an explicit syntax for
exporting "globals" into the including package (this way you can "opt-in"
to bad behavior, rather than it being default)

On Mon, Sep 7, 2015, 10:55 AM James Brantly notifications@github.com
wrote:

Regarding the global keyword, ie global myVar, I feel that is the wrong
way to go about it. Normal definition files today can already declare
globals. Assuming #4668
#4668 was in place, any
definition file could declare globals just like they do today but with much
less chance of conflict with another global. For example, assuming a
promise library that adds a global Promise.

• myprogram
  • mylib
    • promiselib@1.0 // using Proposal: Localized typings for definition files #4668
      Proposal: Localized typings for definition files #4668, the
      Promise global here would not be leaked to myprogram
  • myotherlib
    • promiselib@2.0 // again, not leaked, and therefore no conflict

Note that Promise would not be available to myprogram in this case. If it
did need to be made available, then the program author can do so:

• myprogram
  • mylib ...
  • myotherlib ...
  • promise@3.0 // The Promise global here would be available to
    myprogram

—
Reply to this email directly or view it on GitHub
#4665 (comment)
.

[poelstra]

The "legacy mode" in #4673 mostly addresses the first two points, but still allows global leaks.

@jbrantly Why do you think it allows global leaks? The idea of it is that it puts all these 'globals' (e.g. declare module X and declare module "Y") into a 'temporary namespace' while parsing it, then only makes the contents of the Y module available as the result of the import. All other stuff is never exposed to the global namespace.

Normal definition files today can already declare globals.

Yes, they do. But in most cases, this is not what they really do. It's just basically the only way to make isomorphic typings today. E.g. current bluebird typing exposes a 'global' Promise type in its typing, but when you import BBPromise = require("bluebird");, there's no 'real global' Promise available. (new Promise(...) fails, new BBPromise(...) works.)

So, I consider the Promise type in this example an 'accidental global' (maybe not the best naming :)), not a 'real' one.

However, suppose I do something like import "polyfill-promise";, where polyfill-promise does something like require("es6-promise").polyfill();. Now, polyfill-promise really makes a global Promise available. Anywhere in my program, regardless of explicitly importing polyfill-promise, I can now suddenly new Promise(...), and it works.

That last variant is where I proposed that global keyword for. Because if polyfill-promise's typing would be of the proper external variant, there's no way for it to state that it really makes this 'real' global available. This way, the compiler can complain if two packages try to make the same global available (with incompatible structural types), as this is really going to break at runtime.

Or maybe better, if a package actually tries to use such a global in a way that is not allowed by all definitions of that global. E.g. var p = new Promise(...) might still be allowed even if two 'promise polyfills' both declare a Promise, but p.done() fails if only one of them provides that.

[poelstra]

@weswigham Exactly :)

[jbrantly]

@weswigham The problem with what you're suggesting is that it completely disregards the use of libraries as globals in the browser. If all that TypeScript did was node-style modules then I would agree. But library authors (or definition authors) need to provide typings generally both for node-style modules and browser globals. If using proper external modules you would either need to 1) duplicate the definitions between the global and the module versions or 2) write one definition as a global namespace and then reuse it in the module version, but this leaks the global currently. If there was a better way to easily go from module -> namespace (#2018) then this wouldn't really be an issue. For now I've suggested something like #4337 which (inadvertently) works really nicely with my proposal.

I think the real issue isn't that definition files can make globals, but that dependencies of definitions files can inadvertently make globals. This is a subtle difference, but an important one.

Why do you think it allows global leaks? The idea of it is that it puts all these 'globals' (e.g. declare module X and declare module "Y") into a 'temporary namespace' while parsing it, then only makes the contents of the Y module available as the result of the import

I might have misunderstood this point when reading your proposal. It wasn't clear to me that in legacy mode all globals would be isolated, including something like this:

// wtf.d.ts

declare var SomeGlobal: any;

// myutils.d.ts

/// <reference path="wtf.d.ts" />
declare module "myutils" {
  ...
}

If that was your intent, then I think this line in your proposal "Don't expose any declared modules to global module namespace" is essentially a one-line redefinition of my proposal 😄 You'd literally be saying that all of the dependencies of the module are isolated, which is more or less what I'm saying. If that's the case then I think we're actually fairly closely aligned but with some disagreement about whether or not proper external modules are even necessary. BTW, the above example was what I meant when I said that your proposal can still leak globals.

That last variant is where I proposed that global keyword for.

Today using normal definition files you'd just reference "polyfill-promise.d.ts" (either explicitly or through some fancy-pants lookup). No new syntax needed.

This way, the compiler can complain if two packages try to make the same global available (with incompatible structural types), as this is really going to break at runtime.

I think this point is debatable. I don't think it's necessarily true that runtime would break, and this seems like it could be a real PITA to force TS to not throw errors even though you know that the runtime won't break.

[poelstra]

The problem with what you're suggesting is that it completely disregards the use of libraries as globals in the browser.

No, because in my proposal the "legacy mode" trick (bad name by now, "mixed mode", anyone?) is only applied in the commonjs/node-resolution-mode. Browser/AMD mode can still use what suits them best.

I might have misunderstood this point when reading your proposal. It wasn't clear to me that in legacy mode all globals would be isolated, (...) If that was your intent, then I think this line in your proposal "Don't expose any declared modules to global module namespace" is essentially a one-line redefinition of my proposal

Yes, that was my intent. And the few packages that really wanted declare var to create a global even when they're loaded as CommonJS (instead of just being a trick to get the typing to work) would need to be changed to global var or something when used in "mixed mode".

Today using normal definition files you'd just reference "polyfill-promise.d.ts" (either explicitly or through some fancy-pants lookup). No new syntax needed.

Yeah, and that might be the more logical thing to do in this case.
I'm not a fan of a new global syntax myself, and I don't see a real need for it in the (commonJS-)packages I use (because they don't put stuff on global/window). But for the few packages that do, it might be a solution.

I don't think it's necessarily true that runtime would break, ...

Explained why/when this might break in the example in #4668 (comment) (bottom part of the post) and #4665 (comment) (last sentence).

..., and this seems like it could be a real PITA to force TS to not throw errors even though you know that the runtime won't break.

Agreed, which is why I added the "Or maybe better, if a package actually tries to use such a global in a way that is not allowed by all definitions of that global" :)
This is in line with @mhegazy's comment "should not get errors from the package unless they have a way of witnessing it".
With this definition, I think it will only generate an error when there really is one. (Although it will not generate an error if the JS packages also internally use the global in an incompatible fashion, of course.)

[jbrantly]

bad name by now, "mixed mode", anyone?

Agreed. Would like definitions files as they generally are today to still be first-class citizens.

is only applied in the commonjs/node-resolution-mode

I didn't really mean from the consumption perspective. I meant from the authoring perspective. "you would either need to 1) duplicate the definitions between the global and the module versions or 2) write one definition as a global namespace and then reuse it in the module version, but this leaks the global currently"

However, if proper external modules also had the global isolation logic applied then the second option could be used without leaking globals.

Yes, that was my intent

Well then, cool 👍 So I feel like your proposal could maybe incorporate mine to flesh out that section (without the opt-in part). I'll post something in your proposal about this.

Or maybe better, if a package actually tries to use such a global in a way that is not allowed by all definitions of that global

I don't know. I'm still on the fence about this. It's obviously impossible to know which global was actually applied from a typing perspective. I lean toward thinking it's best to just let the user control which global to apply by referencing the proper definition file rather than trying to figure out some fancy logic to merge the globals and try to warn, etc. shrug Not the hill I'll die on though.

[heycalmdown]

Is there anyone who are using this feature in a serious way? How are you handling the redefinition errors, not even the version conflict you are discussing here?

[jbrantly]

This feature isn't really implemented yet (I don't believe) except for #4738.

It seem like the work so far is just focusing on not allowing triple-slash references nor declare module "a".

[mhegazy]

since we needed to do this right, so for 1.6 @vladima added a check to ensure that any exported module from a node package through typings property is a proper external modules (#4738), and is in fact a .d.ts file (addressed #4667). We need to come up with the full solution for the next release.

[heycalmdown]

There are very small chances to make a "proper" external module without relying on any other NPM packages that is not authored by typescript users. Things like lodash, mongoose, redis, and surely node.d.ts.

@mhegazy So, is #2338 going to be TS 1.7 to solve that problem you think?

[weswigham]

I'm about to be way more verbose than I've been in this issue and the related competing proposal in the past.

@jbrantly

@weswigham The problem with what you're suggesting is that it completely disregards the use of libraries as globals in the browser. If all that TypeScript did was node-style modules then I would agree. But library authors (or definition authors) need to provide typings generally both for node-style modules and browser globals.

That's precisely why we need to find a way to continue allowing ambient external declarations while continuing to improve the package consumer use case. This is why I suggest package-scoping ambient declarations by default. When using .d.ts files in a browser context, this doesn't come up as a problem because you're not using packages! 😄 All ambient declarations are global scoped when there are no packages! Nothing we're discussing here will ever effect browser or flat project consumers. Unless you import "foo" and "foo" is a node package, none of the logic or issues discussed here should ever come into play. Nor those discussed in any of these related proposals.

Here's what I think. People want to reuse their handwritten browser definitions. We should allow ambient module declarations of the style
foo.d.ts:

declare module "foo" {
   //...
}

Within packages. And we should continue to disallow directly having ambient definitions like that at the entrypoint specified in the "typings" field of a "package.json". However, we could then allow (or encourage) the following:
index.d.ts:

/// <reference path="foo.d.ts"/>
export * from "foo"; //pulls the "foo" package from the package scope and exports it to the caller

(First, re-enabling triple-slash references in packages.) And all ambient declarations made within a package are scoped to only within that package - this way when you include a package, you only get members intentionally exported by the package. The ambient module "foo" isn't accessible to your child packages or your parent package - it's as if you declared the types inline in your file. That's the key here. This way my package can include and declare any number of .d.ts files for it's own internal consumption (so I can use underscore or rxjs with types internally or whatever), but then only expose to the package consumer those types which my package actually exposes via its JS.

Intentional modifications to the global scope aren't currently possible within external modules - that's a separate but related issue. It's also a comparatively rare use-case within packages, and is lower priority than allowing reuse of existing .d.ts files with the "typings" field and preventing typing conflicts.

I am not a believer in having "version overrides" for .d.ts files. First off, if I'm distributing typings in my packages, my packages are already versioned. My .d.ts files should not duplicate that information. Secondly, it doesn't solve the problem. If I include two packages which depend on ambient types from the same utility library, and that library has had a breaking change in API between the included versions, my project will fail to compile, despite not interacting with/caring about it at all. (This could be changed by not typechecking included modules but that's a slightly different but related issue.) Additionally, this artificial conflict doesn't indicate any runtime problems, since this configuration is perfectly valid in the node package model. This "problem" only exists in typescript's type-checker's flawed understanding of the packaged world. Packages have per-file scopes in node. External modules are capable of capturing that, which is why the problem does not arise when only using external modules. Our issue is one of reuse - we want to be able to reuse our ambient module declarations (which don't capture these scoping constraints) in package contexts while maintaining package semantics. I see the reliable way to do so is to isolate ambient declarations to within their package's scopes. In this way, a package maintainer only has to manage typings around their own package - not their dependencies' dependencies typings, or their consumer's.

If we accept package scopes as a possible solution we come to the problems with implementing this solution. This requires some somewhat serious work within the type checker, from what I'm told. We don't actually have a concept of a package scope - we have a global scope and file/block scopes. External modules work because we rely on file scopes to isolate everything. To add package scopes, we'd need to add another scope concept between globals and files, and it would only exist by inference (if a package is accessed via an import statement which reads a typings field from a package.json, then inject all global files accessed via that file into a package scope for that package instead, and any external modules inside that package scope access that package scope before the global scope when looking up global types). This is nontrivial by comparison to the proposed version tag, which is why it's met with some resistance.

Undoubtedly, the way forward is to mirror your source module JS files (which are external modules) with external module .d.ts files - this will never have problems - but is cumbersome and usually requires some nontrivial rewriting of existing .d.ts files. It also gives you the issue of needing to maintain completely separate typings for the browser, where you want ambient module syntax, and for node.

[mhegazy]

The check in #4738 is to limit library authors from publishing libraries that may cause conflicts in the global namespace in the future.

We have spent the last two design meeting discussing this issue. we have not managed to come up with a reasonably simple scoping rules that would fit the node semantics and not break existing declaration files.

The main problem is majority of the .d.ts files as they are written today do not express intent of global pollution correctly. if we limit them all to a scope, what does that mean to mutations ot built in types, e.g. Array? do we have a copy of lib.d.ts for every module? should we allow these to be global pouters? what if they conflict...

The ideal world is one where every package carries its own typings with it, and everything is an external module. but we are not there, and we need a way to allow package authors to take a dependency on the lodash's, the mongoose's etc..

I think the original proposal is so far the simplest and most realistic approach to handle this issue without either rewriting all definitelytyped, or the TypeScript compiler or both.

We will try to get this in master as soon as possible to allow for easier package typing. we also need to document publishing typings. @vladima started a wiki page here: https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages, but there is more to be done.

[jbrantly]

@weswigham Thanks for detailed comments.

And all ambient declarations made within a package are scoped to only within that package

I feel like we're pretty much on the same page, actually. I agree with everything you've said with the only possible caveat being...

Intentional modifications to the global scope aren't currently possible within external modules - that's a separate but related issue. It's also a comparatively rare use-case within packages, and is lower priority than allowing reuse of existing .d.ts files with the "typings" field and preventing typing conflicts.

I think they should be possible (because it's possible in JS-land) and that it's something we do need to figure out sooner rather than later since a large part of the point of this discussion is how to handle global conflicts.

FWIW, #4668 pretty much spells out what you've said with two exceptions:

1. Proposal: Localized typings for definition files #4668 is an opt-in mechanism at the .d.ts level ("definition-file scoped") for backwards compatibility but you think it should be an automatic default for the definition files which are resolved during node resolution ("typings" field, etc). What you're saying effectively makes it "package-scoped", as you said. I'm personally on board with this and actually mention it in the proposal.
2. Proposal: Localized typings for definition files #4668 allows for globals to be specified in the node-resolved definition file (basically you aren't required to use a proper external module). But it seems like this ship has already sailed with the latest PRs.

This is nontrivial by comparison to the proposed version tag, which is why it's met with some resistance.

Yea, I figured as much.

It also gives you the issue of needing to maintain completely separate typings for the browser, where you want ambient module syntax, and for node.

I think this is really my biggest gripe (see #4337). I really hope that whatever solution is arrived at takes this into consideration.

@mhegazy

We have spent the last two design meeting discussing this issue. we have not managed to come up with a reasonably simple scoping rules that would fit the node semantics and not break existing declaration files.

Did you look at #4337 or the "mixed-mode" concept in #4673? If so, do you have any feedback on those?

The main problem is majority of the .d.ts files as they are written today do not express intent of global pollution correctly. if we limit them all to a scope, what does that mean to mutations ot built in types, e.g. Array? do we have a copy of lib.d.ts for every module? should we allow these to be global pouters? what if they conflict...

It seems to me like #4337 mostly addresses this. It's opt-in and therefore backwards compatible.

Limiting mutations to built-in types is a good thing unless the the application author wants to see those mutations, in which case they simply use the correct definition file which makes the mutations. Super simple (non-working) example:

import { polyfill } from 'es6-promise'; // note that the definition file here does *not* make the global Promise
import 'es6-promise/polyfill'; // make the global in TS-land, could also be a tripleslash reference or in tsconfig.json
polyfill(); // make the global in JS-land

or, for something like whatwg-fetch which really does just always make a global because it's browser-based:

import "whatwg-fetch";
fetch(...);

Note that if using a package-scoping concept, if whatwg-fetch was used for some reason as a dependency of a package it would not pollute that global down to the application.

rewriting [...] the TypeScript compiler

¯_(ツ)_/¯

[mhegazy]

It seems to me like #4337 mostly addresses this. It's opt-in and therefore backwards compatible.

As guidance that would solve the issue, the problem is not enforceable by the compiler. and what i worry about is making it easier to create these types will leave end users to suffer.

[jbrantly]

I saw #4877 and thought it related to this discussion a bit as it's a node module that creates a global.

[mhegazy]

this does not seem to be needed now with the move to @types. closing.

[jamir0]

Hi, im not pretty sure what im' writing in correct issue, but i guess i have related issue.
in project i use several libs which in dependencies have isomorphic-fetch & whatwg-fetch. after update to typescript@>=2.1.5 i recieve errors:

Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(6,14): error TS2300: Duplicate identifier 'RequestContext'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(14,14): error TS2300: Duplicate identifier 'RequestMode'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(15,14): error TS2300: Duplicate identifier 'RequestCredentials'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(16,14): error TS2300: Duplicate identifier 'RequestCache'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(20,14): error TS2300: Duplicate identifier 'ResponseType'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(22,14): error TS2300: Duplicate identifier 'HeaderInit'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(23,14): error TS2300: Duplicate identifier 'BodyInit'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(24,14): error TS2300: Duplicate identifier 'RequestInfo'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(41,15): error TS2300: Duplicate identifier 'Headers'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(60,15): error TS2300: Duplicate identifier 'Body'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(81,15): error TS2300: Duplicate identifier 'Request'.
Error - typescript - node_modules\@types\isomorphic-fetch\index.d.ts(115,13): error TS2300: Duplicate identifier 'fetch'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(6,15): error TS2300: Duplicate identifier 'Request'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(29,6): error TS2300: Duplicate identifier 'RequestContext'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(37,6): error TS2300: Duplicate identifier 'RequestMode'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(39,6): error TS2300: Duplicate identifier 'RequestCredentials'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(40,6): error TS2300: Duplicate identifier 'RequestCache'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(48,15): error TS2300: Duplicate identifier 'Headers'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(59,15): error TS2300: Duplicate identifier 'Body'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(69,15): error TS2300: Duplicate identifier 'Response'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(82,6): error TS2300: Duplicate identifier 'ResponseType'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(90,14): error TS2300: Duplicate identifier 'HeaderInit'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(91,14): error TS2300: Duplicate identifier 'BodyInit'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(92,14): error TS2300: Duplicate identifier 'RequestInfo'.
Error - typescript - node_modules\@types\whatwg-fetch\index.d.ts(98,13): error TS2300: Duplicate identifier 'fetch'.

i can't remove one of such definitions because they are dependencies from third part packages and they are installed (@types/isomorphic-fetch, @types/whatwg-fetch) own td's.

ps. i used some packages from MS to create webparts with react and apollo-client to connect to graphql.

Please does anyone know how to resolve this conflict?