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

Compiling with -d should emit triple-slash lib paths #28195

Closed
3 of 4 tasks
bcherny opened this issue Oct 28, 2018 · 4 comments · Fixed by #57681
Closed
3 of 4 tasks

Compiling with -d should emit triple-slash lib paths #28195

bcherny opened this issue Oct 28, 2018 · 4 comments · Fixed by #57681
Labels
Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@bcherny
Copy link

bcherny commented Oct 28, 2018

Search Terms

Triple slash, reference, path, lib, library, types, typings, dom

Suggestion

When compiling a .ts file with declaration: true, TSC should emit triple-slash reference paths for all libs consumed by the .ts file.

Problem

Say I publish typings for my package foo to NPM, and foo depends on DOM typings. I then include foo in my project (npm install foo). There are a few things that can happen:

  1. If foo's typings include <reference lib="dom" />:
  • foo has access to DOM
  • My project has access to DOM types (is this sort of leakage desirable?)
  1. If my project includes dom (or uses the same triple-slash directive as in 1):
  • foo has access to DOM
  • My project has access to DOM types
  1. Neither my project nor foo include <reference lib="dom" />:
  • foo errors wherever it uses DOM types (but I have to inspect node_modules/foo/index.d.ts to see those errors)

  • My project treats foo's DOM types as anys (should this error in strict mode?)

    // foo.d.ts
    export function a(): HTMLInputElement
    
    // myProject/index.ts
    import {a} from 'foo'
    a() // any

Solution

If tsc -d generated triple-slash directives for foo/index.d.ts, we can ensure that we're always in case (1). This is nice because if my project doesn't use dom, I shouldn't have to declare it for the sake of foo like I did in (2); it also avoids the any from (3).

// Before
export function a(): HTMLInputElement

// After
/// <reference lib="dom" />
export function a(): HTMLInputElement

An open question is how strict should TSC be? Ie. If foo uses dom but fails to reference it, should this cause a compile error in my project (and a nice message suggesting I add a reference to dom)?

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@kitsonk
Copy link
Contributor

kitsonk commented Oct 28, 2018

While you don't think this would be a breaking change, it could actually be a breaking change. Referencing a lib changes the behaviour of the way that TypeScript's compile context works, and it is then something that can't be easily overridden downstream. If you are running or compiling TypeScript in a situation where you are targetting a non-browser context, like Node.js or some sort of embedded system, and some random .d.ts drags in the dom lib, you could have lots of runtime issues with the code. This is something that would be surprising given the current behaviour.

Also, there are always other ambient types which are not reflected in .d.ts file, like anything in @types. So this would only solve part of the problem of the dependency.

My opinion is that it really is up to the author to determine if these need to be present in the files.

@bcherny
Copy link
Author

bcherny commented Oct 28, 2018

@kitsonk I left "This wouldn't be a breaking change" unchecked on purpose 😄

@kitsonk
Copy link
Contributor

kitsonk commented Oct 28, 2018

DOH! Sorry... 😜

@weswigham weswigham added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Oct 29, 2018
@Luke-zhang-04
Copy link

Not sure why this issue is dead but it would've been really nice to see in Typescript 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants