Skip to content
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

Do not emit /// <reference types=".." /> for augmented declarations in dependencies #11849

Closed
mhegazy opened this issue Oct 26, 2016 · 6 comments · Fixed by #11872
Closed

Do not emit /// <reference types=".." /> for augmented declarations in dependencies #11849

mhegazy opened this issue Oct 26, 2016 · 6 comments · Fixed by #11872
Labels
Bug A bug in TypeScript

Comments

@mhegazy
Copy link
Contributor

mhegazy commented Oct 26, 2016

/// <reference types="node" />
interface I {
    e: Error;
}
> npm install @types/node

> tsc --d a.ts

> type a.ts

/// <reference types="node" />
interface I {
    e: Error;
}

Error should not be cause a type dependency as it partially came from lib.d.ts

@jayphelps
Copy link

jayphelps commented Nov 15, 2016

RxJS member here, any ideas how we can work around this as a library author, so our users don't have to? (as we wait for 2.1) ReactiveX/rxjs#2112 It looks like we misunderstand what the real problem is--we thought we fixed it in ReactiveX/rxjs#2120 but turns out we didn't.

Since I don't see hundreds of devs yelling in this thread about this, perhaps there is something we're doing we can avoid? We could build RxJS with canary/dev release, but we'd prefer not to for risk of adding other bugs as well.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 15, 2016

you can strip out the /// <reference types ... /> from the resulting .d.ts files, wouldn't this be sufficient?

@jayphelps
Copy link

@mhegazy oo just go in by hand and remove them from the artifacts, before we publish? A crude but hilariously effective solution.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 15, 2016

you just need a regex.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 15, 2016

alternatively 2.1 RC should have the fix for the issue.

@rightisleft
Copy link

Sadly using @jayphelps approach to fixing this bug until next RC ....

tetsuharuohzeki added a commit to tetsuharuohzeki/rxjs that referenced this issue Dec 8, 2016
BREAKING CHANGE: TypeScript definitions are now for TS 2.1 and higher.

Abstract
----------

- This change does not cause any breaking change directly,
  but we would causes some breaking change by using new features
  intoduced by v2.1 for the future.
- If we upgrade typescript to v2.1, there are some pros/cons:

\### pros

- we can switch to npm:@types type dependencies by
  microsoft/TypeScript#11849 is fixed in v2.1.
- we can use more powerfull features introduced by v2.1.
  - We can use [tslib](https://github.com/Microsoft/tslib)
    to reduce our compiled code size.
- we don't have to upgrade major version v5 stable release
  if we merge this now before v5 stable release.

\### cons

- This would introduce some breaking change for the future.
  - But we might cause some breaking change intentionally because
    we don't lock the range of our typescript version to patch version.
    This might causes to accept new features which uses the lang
    features introduced by a latest version of TypeScript

Relations
-----------

- [TypeScript 2.1 RC: Better Inference, Async Functions, and More](https://blogs.msdn.microsoft.com/typescript/2016/11/08/typescript-2-1-rc-better-inference-async-functions-and-more/)
- [Announcing TypeScript 2.1](https://blogs.msdn.microsoft.com/typescript/2016/12/07/announcing-typescript-2-a1/)
@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.
Labels
Bug A bug in TypeScript
Projects
None yet
4 participants