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

Better default experience when using a tsconfig file with no "files" property. #3289

Closed
jbrantly opened this issue May 28, 2015 · 11 comments
Closed
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@jbrantly
Copy link

When using a tsconfig file with no files property all declaration files under node_modules are currently pulled in. This is especially problematic if typescript itself resides under node_modules since you wind up with duplicate identifiers from lib.core.d.ts and lib.core.es6.d.ts, etc. I realize this is partially taken care of with #3043 and #3188 but I'm wondering if there should be a special default case of excluding typescript declaration files. Plugins for JS toolchains like gulp-typescript and ts-loader pull in typescript locally so this becomes an issue there.

Edit: Repro steps for TS 1.5 beta

  1. mkdir test
  2. cd test
  3. npm install typescript (install locally, not globally)
  4. touch tsconfig.json
  5. ./node_modules/typescript/bin/tsc
@mhegazy
Copy link
Contributor

mhegazy commented May 28, 2015

We have talked about defaults as part of the "exclude" support; it was not clear what are the rules that qualifies something to be in the default excludes, e.g. node has node_modules, bower has bower_components, then there are editor specific .vs, .settings, .config...etc.. the other issue is it can be confusing to debug why a folder is excluded while others are not, if there are implicit defaults.

Having said that, i would be curious what others think about it as well.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label May 28, 2015
@jbrantly
Copy link
Author

I understand and agree. Excluding all of node_modules by default might be a bit broad, and I wasn't suggesting that. I was only suggesting that TypeScript lib files be excluded by default for 2 reasons:

  1. There is a clear error if all TypeScript lib files are included by default
  2. I believe the appropriate TypeScript lib files will get pulled in anyways (--noLib)

@mhegazy
Copy link
Contributor

mhegazy commented May 28, 2015

sorry miss understood the intent. The problem with library files is how do you know them.. the name is not sufficient. There are two ways you can specify a lib file, one is --noLib and include the lib file, the second is you add a comment to the top of the file /// <reference no-default-lib="true"/>, so we can not really know if this the lib.d.ts you want or not excluding library files is a bit arbitrary.

in your case, would not specifying "exclude" : ["node_modules" ] in tsconfig.json be sufficient?

@jbrantly
Copy link
Author

so we can not really know if this the lib.d.ts you want or not excluding library files is a bit arbitrary

I realize support for globs isn't there, but assuming it was for a moment, something like node_modules/**/typescript/bin/*.d.ts would be sufficient to identify TypeScript lib files that should be excluded. Keep in mind when I say excluded I don't mean "these never, ever get included in a program". I just mean they don't come in as part of the scanned files from tsconfig.json. For example, if you have a program today that has a tsconfig.json file with a files property that does not include lib.d.ts it still (I believe) gets pulled into the program by default. So since this is the case, it's redundant to also include lib.d.ts in the scan (and in fact causes errors since lib.es6.d.ts gets pulled in, etc).

in your case, would not specifying "exclude" : ["node_modules" ] in tsconfig.json be sufficient?

Yes, but I don't think that's going to happen by default, right? I'm trying to come up with a solution that's narrow enough that it could be enabled by default.

@jbrantly
Copy link
Author

I added some simple repro steps to the OP, just for reference.

@vvakame
Copy link
Contributor

vvakame commented Sep 6, 2015

👍
exclude is not supported something likes ! glob pattern.
I want to use lib.es6.d.ts with --target es5 too.
related? #3232

@mhegazy
Copy link
Contributor

mhegazy commented Sep 6, 2015

@vvakame, we are working on the glob support. i am not sure i see how lib.es6.d.ts is related to this discussion.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 6, 2015

This is addressed with #3188. closing.

@mhegazy mhegazy closed this as completed Sep 6, 2015
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Sep 6, 2015
@jbrantly
Copy link
Author

jbrantly commented Sep 6, 2015

FWIW, #3188 does not fix this. For instance, you can still run the repro steps using the latest nightly. This issue wasn't about having the ability to exclude stuff, it was about having a very narrow set of files automatically excluded by default.

That said, I could understand a WONTFIX resolution.

@danquirk danquirk reopened this Sep 8, 2015
@danquirk
Copy link
Member

danquirk commented Sep 8, 2015

Re-opening for a little more discussion given the latest clarification. I think this falls into some broader issues around managing dependencies, especially duplicated ones. The new 'tsc init' command is another way we could alter the default experience for people if there's a reasonable thing to do there.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 8, 2015

We talked about adding implicit exclude values in the past. The idea of an exclusion policy that is opaque to the user was not appealing. moreover, what should be in it, is it node_modules? what about bower_components? is it only the typescript package, so what about other packages that have their own version of the library, e.g. angular2 for instance? what about .vs or .settings or all the folders with . prefix? and custom editor/build folders that generate typescript resources.

as @danquirk mentioned, --init is mainly targeting these scenarios.

@mhegazy mhegazy closed this as completed Dec 9, 2015
@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
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants