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

Remove "files" from tsconfig.json #175

Closed
vivainio opened this issue Dec 21, 2015 · 14 comments
Closed

Remove "files" from tsconfig.json #175

vivainio opened this issue Dec 21, 2015 · 14 comments

Comments

@vivainio
Copy link
Contributor

It's considered a bit of an antipattern to have list of source files in tsconfig.json.

Currently, you need to add your new .ts files there to get vscode working. This doesn't scale to nontrivial projects

https://github.com/Microsoft/TypeScript/wiki/tsconfig.json :

If no "files" property is present in a tsconfig.json, the compiler defaults to including all TypeScript (*.ts or *.tsx) files in the containing directory and subdirectories. When a "files" property is present, only the specified files are included.
@PatrickJS
Copy link
Owner

can you make a pull-request and also set rewriteTsconfig to false

@vivainio
Copy link
Contributor Author

I tried just removing those, but the project failed to build after that - for webpacky reasons I'm not familiar with (apparently proceeding to compile everything, as opposed to only files that can be discovered by crawling dependencies starting from app.ts, as tsify is doing)

@PatrickJS
Copy link
Owner

I removed the files but now I have to exclude everything :/ I also ran into a problem with typings and protractor's ambient type definition file.

cc @blakeembrey
what do you recommend?

@vivainio
Copy link
Contributor Author

For reference, here is all we have in our tsconfig.json (but as said, we are using tsify):

{
    "compileOnSave": false,
    "compilerOptions": {
        "module": "commonjs",
        "sourceMap": false,
        "moduleResolution":"classic",
        "target":"es5"
    }
}

It's not next to our node_modules, but in a directory that contains our apps ts code. In the angular2-webpack-starter, this would be either src/ or src/app

@PatrickJS
Copy link
Owner

actually it does make more sense to have tsconfig.json in src/ and another one in test/ then we can omit files without having to worry about different environments

@blakeembrey
Copy link
Contributor

I've never seen anything about "files is an anti-pattern". Nor have I seen the issue in VSCode that is described. What are you trying to fix? The file won't be picked up until it's either imported or included in the files array, but I wouldn't say that makes files an anti-pattern (I'd love some data on that, since it works as expected - their basically entry points to the application for compilation).

@vivainio
Copy link
Contributor Author

It looks like this before the file is imported somewhere:

image

Empty files section is used as starting point e.g. here:

"Entry point" for collecting files should be app.ts (or at least it's with tsify). If dependency tree from app.ts is sufficient (don't know enough/anything about webpack to know for sure TBH), the files section seems redundant, no? It's certainly a bit of a pain to maintain (think merge conflicts when everyone is adding new files, etc)

@blakeembrey
Copy link
Contributor

I'm not sure if you missed what entry point means, but files doesn't include every single TypeScript file you manually added to the project. It's the entry files to start compilation. And yes, that's the error message because you haven't imported it, once you import it it'll be included in the project. The reason you get that error is because it's not referenced or included anywhere. Honestly, I wouldn't drop using the files array for any reason, even using exclude can be a pain in the ass.

@blakeembrey
Copy link
Contributor

I didn't understand any of the last paragraph, sorry. What are you trying to say there? Files is used by the TypeScript compiler and tooling. Both the TypeScript compiler, tooling, Webpack, tsify, anything will resolve imports as I described above, but the compiler and tooling doesn't use tsify, it uses tsconfig.json. Sure, you can omit files, but then you have to use exclude instead otherwise you might end up compiling something outside the scope of the project. You end up with a blacklist instead of a whitelist. Whitelists seem easier to maintain here, since compilation will be explicit instead of implicit.

@vivainio
Copy link
Contributor Author

@blakeembrey currently tsconfig.json includes (redundand?) files that are referenced from app.ts:

 "files": [
    "src/app/directives/x-large.ts",
    "src/app/providers/title.ts",
    "src/app/home/home.ts",
  ],

app.ts:

import {Title} from './providers/title';
import {XLarge} from './directives/x-large';
import {Home} from './home/home';

If those are removed from tsconfig.json (meaning that the user will not end up maintaining the file most of the time), that would be fine as well.

Visual Studio Code understands tsconfig.json doesn't have "files" property, as said it understands all .ts (and .d.ts) files contained in the directory tree under the tsconfig file itself. Don't know about Atom.

@blakeembrey
Copy link
Contributor

Yes, seems redundant to me too. But I'm not the person to fix this, just responding because I was pinged unceremoniously. @gdi2290 Fix your shit. The other option is using exclude and basically doing the inverse, but I prefer things to be explicit instead of implicit anyway - includes should land in tsconfig.json soon which will resolve this forever.

Yes, Atom and Sublime and other tooling also understands this also. But you have to exclude everything that's not meant to be included and I can tell you that's a pain sometimes and I wouldn't recommend it as the default. At a minimum, you'll have to start with node_modules. Also, you can't use files with exclude, which means the second some file inside of an excludes needs to be included (E.g. a module provides you an ambient declaration, such as reflect-metadata) you can no longer use exclude properly.

Edit: Grammar.

@vivainio
Copy link
Contributor Author

I made a PR that removes redundant files (making this less confusing), #176. Stuff still seems to work (at least on vscode, didn't try with Atom).

bootstrap.ts is a sufficient starting point for the app itself

@PatrickJS
Copy link
Owner

I would have to exclude node_modules and typings for it to work since there is a problem with protractors type definition

@PatrickJS
Copy link
Owner

entry files are now the only files other than spec files which needs to be fixed too. Closing for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants