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

New APIs added to lib.d.ts may break client codes. Allow duplicated members in interfaces? Make lib.d.ts overridable? #3215

Closed
duanyao opened this Issue May 19, 2015 · 42 comments

Comments

Projects
None yet
@duanyao
Copy link

duanyao commented May 19, 2015

I noticed that recently a lot of new APIs are added to lib.d.ts. Of cause this is a good thing, however my code breaks badly because of such update.

Previously I had added a lot of declarations in my code for HTML5 APIs that were missing in lib.d.ts . Now some of them are added to lib.d.ts, and I get quite a few "duplicated identifier" errors. For example, I had added this piece to play with fullscreen:

interface Document {
//...
  fullscreenEnabled: boolean;
  mozFullScreenEnabled: boolean;
  webkitFullscreenEnabled: boolean;
}

Now fullscreenEnabled and webkitFullscreenEnabled are available in lib.d.ts, and my code breaks, and I have to remove them. However, you are still missing mozFullScreenEnabled right? -- well, waiting for the next break.

Web platforms are constantly evolving, it is not practical to expect lib.d.ts always up to date, so developers just have to write such compensating declarations from time to time. Can TS avoid such breaks? I have a few ideas:

  • Allow duplicated members in multiple interface declarations, as long as they are identical. In the fullscreen API example above, tsc should not fire errors. Optional compiling warnings are acceptable, to assist developers to remove redundant codes at convenient time.
  • Make lib.d.ts overridable. When tsc find conflictions between lib.d.ts and client codes, it should pickup declarations in client codes. This is because declarations lib.d.ts may contain bugs, developers should be allowed to workaround them. E.g. MutationObserver's constructor was missing a parameter for a long time. Again, optional warnings are welcomed.

What do you think?

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented May 19, 2015

Interfaces are open-ended... Once you strip out the collisions, you will only be left with the "extensions" to the interface you are making. For example, this is perfectly valid TypeScript:

interface Foo {
    bar: boolean;
}

interface Foo {
    qat: boolean;
}

var foo: Foo = {
    bar: false,
    qat: true
};
@duanyao

This comment has been minimized.

Copy link
Author

duanyao commented May 19, 2015

Interfaces are open-ended -- I known.
The problem is: why should duplicated members in multiple parts of an interface be treated as error? I think duplications are perfectly legal if an interface is developed/maintained by multiple parties independently.

Web APIs is a good example. Different browser vendors implement their own set of APIs. Largely the same, but have many small differences, e.g. vendor-prefixed APIs . Currently, TS's lib.d.ts is based on IE's API, this is obviously limited. Ideally, every browser vendors should provide their own version of lib.d.ts, and developers can include some or all of them to achieve cross-platform-ness. An imaginary client code looks like:

///<reference path='lib.ms.d.ts'/>
///<reference path='lib.webkit.d.ts'/>
///<reference path='lib.blink.d.ts'/>
///<reference path='lib.moz.d.ts'/>

var requestFullscreen = document.requestFullscreen || 
  document.mozRequestFullScreendocument || document.webkitRequestFullscreen || 
  document.msRequestFullscreen;
//...

But obviously this can't be done with current TS, because those lib.nnn.d.ts files contain large amount of duplicated members of interfaces.

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented May 19, 2015

This is can be a giant pain. We should try to figure something out.

@basarat

This comment has been minimized.

Copy link
Contributor

basarat commented May 20, 2015

Make lib.d.ts overridable. When tsc find conflictions between lib.d.ts and client codes, it should pickup declarations in client codes.

FWIW this is allowed when compiling with --nolib flag.

Feel like recommending that lib.d.ts be removed from core and spinned into an external project maintained by MS + community.

@duanyao

This comment has been minimized.

Copy link
Author

duanyao commented May 20, 2015

FWIW this is allowed when compiling with --nolib flag.

No, I just want to override the conflicting pieces, not lib.d.ts entirely. Maintaining a whole lib.d.ts myself is not practical (and there are lib.d.ts used by IDEs).

Feel like recommending that lib.d.ts be removed from core and spinned into an external project maintained by MS + community.

Good idea. However it is still nice to allow duplication/overriding of declarations, because there is still no guarantee that individual parties that maintaining those declarations keep in sync with each other.

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Jun 3, 2015

Another use case for this: if a project wants to target both ES5 and ES6, they have to include polyfill declarations for ES5 emit (eg. Promise) but change the build to remove these for ES6 emit. It would be easier if both es6-promise.d.ts and lib.es6.d.ts could be present to the compiler, so long as they don't conflict.

@ENikS

This comment has been minimized.

Copy link

ENikS commented Jun 5, 2015

There should be no harm in multiple declarations of the same interface as long as there are no conflicts.
Another option would be conditional compile, something along these lines:
#ifdef Promise
...
#elseif
...
#endif
This needs to be resolved soon, it is getting really painful to target both es-5 and es-6

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jun 6, 2015

This needs to be resolved soon, it is getting really painful to target both es-5 and es-6

What is wrong with just including lib.es6.d.ts in your project? the library is a super set of ES5 one, and if your project targets both, and you have the correct pollyfils, then you should be safe.

@ENikS

This comment has been minimized.

Copy link

ENikS commented Jun 6, 2015

I have two different node packages defining Iterable and IEnumerable interfaces. Including lib.es6.d.ts would not solve that problem because it also defines Iterable.
How about these who will add *.d.ts of my package? I can not require them do add lib.es6.d.ts as well.
What is wrong with ignoring duplicates?

@duanyao

This comment has been minimized.

Copy link
Author

duanyao commented Jun 6, 2015

This issue also affects DefinitelyTyped potentially. Currently there are definations for webrtc, web speech, web midi, and firefox/chrome specific apis, etc, which will confilict with future TS releases. It is even harder for web developers to resolve such conflicts. One can't simply drop DefinitelyTyped because lib.d.ts doesn't contains all vendor prefixed APIs.

I think ideally switching ES5/ES6 targets should not involve changing lib.d.ts, and "compatibility note" suggested in #3250 is a better alternative. There are some ES7 APIs to be added to TS soon, do we want yet another lib.es7.d.ts?

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Aug 3, 2015

Write up proposal for decoupled libraries

@ENikS

This comment has been minimized.

Copy link

ENikS commented Aug 24, 2015

Added proposal at #4423

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Aug 25, 2015

@ENikS Proposal:

Allow duplicated members in multiple interface declarations, as long as they are identical. Optional compiling warnings are acceptable, to assist developers to remove redundant codes at convenient time.

This should not generate any errors, example:

shim.d.ts

interface Symbol {
    toString(): string;
    valueOf(): Object;
    [Symbol.toStringTag]: string;
}

es6.d.ts

...
interface Symbol {
    /** Returns a string representation of an object. */
    toString(): string;

    /** Returns the primitive value of the specified object. */
    valueOf(): Object;

    [Symbol.toStringTag]: string;
}
...
@ENikS

This comment has been minimized.

Copy link

ENikS commented Aug 26, 2015

Make lib.d.ts overridable. When tsc find conflictions between lib.d.ts and client codes, it should pickup declarations in client codes. This is because declarations lib.d.ts may contain bugs, developers should be allowed to workaround them. E.g. MutationObserver's constructor was missing a parameter for a long time. Again, optional warnings are welcomed.

Wanted to add clarification for above argument. As @kitsonk explained, allowing duplicate declarations should resolve this issue as well. Consider following:

  • Interface declaration has no duplicates - just extend the interface.
  • New declaration in client code is the same as lib.d.ts (or any other interface) - simply ignore/merge by compiler.
  • (optional) Declaration overrides member with different data type - solved by type unions. (described in #4302)
@glen-84

This comment has been minimized.

Copy link

glen-84 commented Nov 18, 2015

I'm just getting started with TypeScript, and targeting ES6 because I want to use async/await. When I include the typings for whatwg-fetch, it references es6-promise.d.ts, and I get Duplicate identifier 'Promise' (x4).

Is there a way to fix this without manually updating the typings?

I tried to exclude es6-promise.d.ts using tsconfig, but it's still included because it's referenced by the whatwg-fetch typing (and from within tsd.d.ts).

Should the author of whatwg-fetch remove the reference, or should this be fixed by TypeScript itself? I imagine that this will continue to happen with other typings.

I don't really know what I'm talking about, but in JS, if you define the same function twice, the last one will be invoked. Couldn't TS type definitions behave the same way, i.e. the last loaded definition is used? ... this probably makes no sense, but I'm just trying to find a solution.

@helios1138

This comment has been minimized.

Copy link

helios1138 commented Nov 23, 2015

+1

@nathantsoi

This comment has been minimized.

Copy link

nathantsoi commented Jan 19, 2016

suggested workaround for this? DefinitelyTyped/DefinitelyTyped#5015 (comment) maybe?

@ENikS

This comment has been minimized.

Copy link

ENikS commented Jan 19, 2016

As one of the possible workarounds it it OK but it does not really solve the big issue...

@du5rte

This comment has been minimized.

Copy link

du5rte commented Jan 21, 2016

A massive +1, you import angular2 into a project, and BOOM! 1221 Error Lines of TS2300: Duplicate identifier due to node.d.ts and es6-shim.d.ts

As we start working with more and more modules in TypeScript this is enviable. I actually think it's good practice for each module to have all the typings that it works with, but I would propose Cascading Overwriting to solve the issue of duplicated typings.

Priority from bottom to top, :
0. typings in typescript

  1. typings in module tsd.json file
  2. typings found in module source code in cascasting order
  3. typings in original module tsd.json file
  4. typings found in original module source code in cascasting order
  5. typings in project root tsd.json file
  6. typings found in project source code in cascasting order

Examples:
es6-shim.d.ts in typescript has priority 0
es6-shim.d.ts in angular2 has priority 1
es6-shim.d.ts in es6-shim module has priority 3
es6-shim.d.ts in project directory has priority 5

@hesalx

This comment has been minimized.

Copy link

hesalx commented Jan 26, 2016

@monteirocode I would not like such overwriting, it likely goes against TypeScript principles of predictability and obviousness, thus you have strong typing but don't know where the definitions came from without an extra exploration.

Objectively, I'm for ignorance of identical definitions at the first place (#3215 (comment)). It just fixes a bunch of existing issues for many projects and doesn't create new problems or breaking changes (does it?). I would be glad to see this implemented ASAP. IMHO, only after that any extended behavior (type unions, overwriting, etc.) may be discussed.

@alexeagle

This comment has been minimized.

Copy link
Contributor

alexeagle commented Feb 5, 2016

Hi there, I've made a change in Angular 2 to fix the BOOM! of Duplicate identifier. Will be available in beta.4. See angular/angular#5807

I think it's a burden for users to be careful that each typing is only installed once (eg. installing es6-shim.d.ts and es6-promise.d.ts) so I hope there will still be a resolution on this issue.

@mykohsu

This comment has been minimized.

Copy link

mykohsu commented Feb 24, 2016

I have another use case:

There is a third party library that has started creating their own type definitions - unfortunately many of the interface members are just stubbed with type any, which is nearly useless.

Even with the proposal here, I cannot override the provided type definition with the type I know to be correct. The type definition itself can be updated, but in the next version you'll have to start over.

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Apr 11, 2016

Reference Type directives are designed to solve exactly this problem by providing a canonical lookup location for things which mess with the global scope.

Combined with the library decoupling work, I think we're addressing all the scenarios here as best as is feasible. There will need to be some level of restructuring of code that found itself in this state, but fundamentally it is solving the problem where two people both want to declare something in the global scope.

@duanyao

This comment has been minimized.

Copy link
Author

duanyao commented Apr 12, 2016

@RyanCavanaugh Do you mean Library include directives? It is a very useful feature, but I'm not sure how "duplicated identifer" is resolved. Is it still an error to declare an identical member in a global interface in client code?

@basarat

This comment has been minimized.

Copy link
Contributor

basarat commented Apr 12, 2016

It is a very useful feature, but I'm not sure how "duplicated identifer" is resolved

It doesn't. But it allows library declaration authors to not pollute the global namespace. Thus decreases the likelihood of global namespace collision 🌹

@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Apr 12, 2016

It solves it because the offending declaration files can be rewritten to be included via library directives, which will only happen once

@duanyao

This comment has been minimized.

Copy link
Author

duanyao commented Apr 12, 2016

I want to use some DOM4 APIs and installed dom4 typings (typings install -A dom4). What will happen when some DOM4 APIs are added to lib.d.ts in future? E.g.

interface ParentNode {
    children: HTMLCollection;
}

I also note that duplicate method in interface is not an error, but duplicate property is.
E.g.

interface Element {    
    innerHTML: string; // error: duplicate identifer
    getAttribute(name?: string): string; // OK
    getAttribute: (name?: string) => string; // error: duplicate identifer
}

This is a strange divergence to me.

@manish153

This comment has been minimized.

Copy link

manish153 commented Jun 19, 2016

facing same issue in beta17

@kirilpopov

This comment has been minimized.

Copy link

kirilpopov commented Jul 2, 2017

This is really annoying when trying to compile to es6 - and I am not talking about Angular ,it is valid for any TS project (in my case electron application).

If any of your dependencies has types/es6-promise as dependency you get the duplicated Promise error mentioned above.

Now your options are (as far as I can see):

  • delete es6-promise package on pre-build step (ghhhh)
  • define some crazy interfaces and use them instead Promise (2x ghhh)
  • switch to es5 :(
@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Jul 3, 2017

@kirilpopov the other option is to manually exclude whatever the type file that is providing the typing in the tsconfig.json.

@Microsoft Microsoft locked and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.