microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.15k stars 12.51k forks source link

support globs in tsconfig.json files property (or just file/directory exclusions) #1927

Closed JeroMiya closed 8 years ago

JeroMiya commented 9 years ago

An extension to https://github.com/Microsoft/TypeScript/pull/1692

Our ts project uses a grunt-based build system, with bower dependencies for the application and node_modules for build-tools. We will need the ability to exclude node_modules and bower_components from the build by default (unless files are referenced via /// ).

This causes an issue: we have end-to-end tests running in protractor, as well as unit tests running in jasmine, and unless we have the ability to exclude node_modules and bower_components, both will be parsed and we'll get duplicate definition errors.

In Visual Studio today, this is why we need to exclude both bower_components and node_modules from the project, because visual studio automatically parses all files in the project with no way to exclude them except to keep them out of the project altogether.

A number of third party tools have started to support tsconfig.json, and one thing I've noticed is that most of them support some sort of extension to tsconfig.json that supports file globs. I propose we just add support into the spec for file globs. For the version of typescript on npm, we can just depend on the appropriate file glob libraries in the npm ecosystem. For the Visual Studio embedded version of TypeScript, just embed the glob libraries that are needed statically in the installation.

As a fallback, if the notion of including a third party glob library is a non-starter, then the following fallback supporting a subset of the glob patterns would solve our problem at least partially.

Fallback: Support both file and directory inclusions and exclusions in the files property:

files: [
  'app/', (equivalent to 'app/**/*.ts')
  '!e2e/', (equivalent to '!e2e/**/*.ts')
  '!node_modules/',
  '!app/bower_components/',
  '!app/vendor/fooLib/fooLib.ts'
]
danquirk commented 9 years ago

We certainly talked about this for awhile. See Anders' comment here for one large concern: https://github.com/Microsoft/TypeScript/pull/1692#issuecomment-70262537

JeroMiya commented 9 years ago

Yes, I've read that comment. Unfortunately, for larger and more complicated projects, the lack of these glob and exclusion patterns makes it impossible to use tsconfig.json at all. If editors support tsconfig.json, we would end up having to disable it due to various errors in mismatched configuration between e2e, unit tests, and the application. And, if the editor uses tsconfig.json to configure compile-on-save and auto-complete in a standard way (and doesn't support a way to override this in a custom project file format, or extension to tsconfig.json as is the case with atom-typescript for example), then we've lost all of that functionality as well.

Globs and exclusion patterns are just how large typescript projects are made managable. We manage it now with build tools like grunt and gulp which support globs and exclusions, but so far the editors and IDEs haven't been configurable in the same way, leading to messy workarounds. For example, in visual studio we have to exclude all the end to end tests from the main app project so it doesn't parse them and complain about duplicate identifiers. We then have a separate visual studio project which just includes the end to end tests. We should be doing the same thing with unit tests, but we decided not having jasmine type declarations visible from the application code was not worth creating a separate unit test project. I would rather edit files in the project all at once, like I would do in Sublime or Atom or Brackets.

NoelAbrahams commented 9 years ago

@JeroMiya, perhaps the real problem here is how the projects are organised. I would consider moving the tests (both unit and end-to-end) to a separate project(s). IMO it's a better practice to keep tests and application code separate.

JeroMiya commented 9 years ago

There are many style guides out there advocating for grouping files by feature. We've tried it both ways and find that grouping things by feature instead of by file type helps you be more productive, especially for larger projects with hundreds of features. You tend to work on a particular feature at a time, and it's a real pain to track down all the files related to that feature when they're spread out across multiple root directories. This way also encourages developers to write unit and end to end tests at the same time as changes to the code. I highly recommend it over keeping test code separate from the application code.

Regardless, as a compiler, TypeScript should not be opinionated about project file organization.

NoelAbrahams commented 9 years ago

Fair enough. We tried the grouping files by feature before moving the test cases out. Having test code in the same place as application code made it difficult to prototype features, because a lot of time was spent fixing compilation errors in the test cases.

basarat commented 9 years ago

FWIW atom-typescript supports filesGlob as an optional extension to files. See spec.

NoelAbrahams commented 9 years ago

@basarat,

    "filesGlob": [
        "./**/*.ts",
        "!node_modules/**/*.ts"
    ]

That looks like a nice feature, because it actually plays well with "files". I wonder what the objection from the TS team is to including this.

Perhaps the argument is: One cannot post the tsconfig to someone else to say "Hey, here's a description of all the files in my project".

Edit (after morning tea)

quote from @ahejlsberg

(1) Let me drop an empty tsconfig.json file in a directory and have everything in there be a project, and (2) let me describe the exact list of files I want included. The current design covers both with a minimum of complexity. That said, I can certainly see how an "exclude" list could be useful, although I suspect we'd then need globbing as well, so the level of complexity goes up. I definitely don't want this growing into an alternate build system. The intent is more to offer tools (e.g. editors) a simple and deterministic way to decide what the compilation context for a particular file is.

basarat commented 9 years ago

Just a note: whenever atom-typescript reads a file with filesGlob it expands the files in 'tsconfig.json' file so that people that are not using atom-ts will still get the correct list of files.

rcollette commented 9 years ago

+1 Globs are so common in use now. There could be .ts files in bower components under my project which have already been compiled to js and may have been done with a different release of TS so I wouldn't want them recompiling. I guess exclusion would handle the case but it just seems like once you support globs you won't need anything else.

I think leaving this up to an IDE to edit, like atom-typescript, is a bad idea. You're basically asking every IDE to implement the logic to keep the tsconfig.json file up to date.

bidiboom commented 9 years ago

I'm in the same case as @JeroMiya , complexe project (e2e, unit test and project code). The outDir option is really good to this kind of project so we can cleanly separate our TS code from the produce code : no accidental JS code modification by the team for example. And this glob option is almost mandatory for our kind of project due the number of file. Thanks to Gulp, we can handle this point but it can be really good to have this glob supported by the tscong file

panuhorsmalahti commented 9 years ago

Globs are definitely needed. I don't see how I could manage the complex, multi-package TypeScript-based platform at work without globbing. Referencing every file by hand would be madness and require large amount of unnecessary work.

Globbing is especially useful when working with npm. I can depend on a npm package (which includes the type definitions), and then glob them in without knowing about the internals of that package.

Using the current setup in our project I can simply build package A, which consists of multiple modules, then require it from package B and glob the type definitions without any manual work even if the file structure of package A changes.

In theory the package A could output a single type definition file, but this is not supported by TypeScript, as each module of package A will create it's on .d.ts file. I already had to do a bit magic by writing a sort of "compiler" to make the .d.ts files ambient external declaration files automatically.

park9140 commented 9 years ago

@ahejlsberg, @basarat, a few thoughts on this.

One major issue with, .net projects in general, is with source control systems and project files. By inserting a file reference into your project file every time a new file is added to a folder you end up with additional and unnecessary churn in your repository.

On larger projects with multiple teams working on them you constantly end up with conflicting changes to files like tsconfig. As such in addition to unnecessary file churn we now have additional labor to manually merge changes to a file that could be automatically handled.

basarat commented 9 years ago

As such in addition to unnecessary file churn we now have additional labor to manually merge changes to a file that could be automatically handled.

Agreed and would be great to have filesGlob supported.

FWIW If its a merge commit just delete the files section and let atom-typescript regenerate it for you. If its a rebase I chose "keep both" at each level ;). YMMV. Definitely needs glob support.

mhegazy commented 9 years ago

We would be open to taking a PR for excludes folders.

Our problem with globs is the complexity to support it, and we usually do not like to take dependencies on other packages as it limits our interoperability and system-independence. Having said so I think it is an important feature and indead does add value. Would be open to proposals here.

basarat commented 9 years ago

So no for stuff like : https://github.com/aspnet/Home/wiki/Project.json-file#sources ?

FWIW I don't have a big issue with the workaround we've managed :). An updating project file isn't a problem new or annoying to me personally.

johnpapa commented 9 years ago

tldr; Wow, we need this to support glob patterns.

Here is my use case ... I have a project with a few 100 files in TS and several 3rd party libraries with a concatenated lib.d.ts file. I want intellisense everywhere with the least amount of friction.

Option 1) Add /// reference to every file to point to the lib.d.ts and leave the tsconfig's file array undefined so it reads all of my app TS. Downside ... every file has this extra line ... yuk (100's of places)

Option 2) Add a gulp task that creates the files array in the tsconfig.json with the 3rd party lib.d.ts and my .ts files (lots of them). Downside, a gulp task to keep up, but at least my code is untouched

Option 3) Add glob support to tsconfig's files array so I can do this

files: ['lib.d.ts', '**/*.ts`]

I vote for option 3. So much easier and less friction for devs.

dmorosinotto commented 9 years ago

+1 for globs in files with syntax for exclusion files: ["lib.d.ts", "**/*.ts", "!node_modules/**/*.ts"]

elishnevsky commented 9 years ago

I see this issue is still open. Is support for glob patterns in tsconfig.json still being considered? That would make so many developers happy.

CyrusNajmabadi commented 9 years ago

Is there a standard file-globbing specification anywhere? If we do this, we'll need to ensure that the globbing works the same on unix/windows, and that likely means writing the code to understand it ourselves.

danquirk commented 9 years ago

Ideally we could just use some existing thing like https://www.npmjs.com/package/glob rather than rolling our own :(

CyrusNajmabadi commented 9 years ago

Ideally. But can we use such packages from tsc.exe where we don't have access to node packages?

ahejlsberg commented 9 years ago

Right, the issue is that tsc.exe and the VS language service aren't in a position to take a dependency on node.js and therefore we can't use the node.js glob package. My feeling is that our best option so far is what is suggested in #3043: An exclude property in tsconfig.json that specifies a list of files and folders to exclude when a files list isn't explicitly specified. The net effect is the same as globbing with the list ["**/*.ts"] followed by a bunch of excludes. For example

{
    exclude: ["node_modules", "tests"]
}

would be equivalent to the globbing pattern ["**/*.ts", "!node_modules/**/*.ts", "!tests/**/*.ts"].

poelstra commented 9 years ago

Is there a standard file-globbing specification anywhere?

@CyrusNajmabadi There's the well-known .gitignore, although that of course is the 'inverse' of e.g. Atom Typescript's filesGlob.

@danquirk That package seems to have problems completely excluding a directory (see e.g. https://github.com/TypeStrong/atom-typescript/issues/332), which is a must for excluding large node_modules folders. Additionally, it has had some breaking changes in the way it interprets its globs, and is apparently going to remove the negate-option.

CyrusNajmabadi commented 9 years ago

Dan mentioned another possibility would be for us to just include glob.js in our compiler (if it was appropriately licensed). Then we would at least have the same syntax/behavior as an extremely popular package in the node space.

poelstra commented 9 years ago

@ahejlsberg Note that a pattern like "!node_modules/**/*.ts" would indeed lead to the same end-result, but will likely still traverse the full node_modules tree (because of the explicit *.ts at the end). Better would be e.g. just "!node_modules/" (or "!node_modules/**", if it detects that as a prefix-pattern).

ahejlsberg commented 9 years ago

Just put up #3188 that adds support for "exclude" in tsconfig.json.

ejsmith commented 9 years ago

Exclude is not enough. Almost all other modern dev tools are supporting globbing. How can I say that I want all files from 2 folders and not include anything else without having to list individual files?

inongogo commented 9 years ago

I think there is another use case which will not be fulfilled with an exclude option: ["**/*.ts", "!**/*.spec.ts"]

ejsmith commented 9 years ago

Yes, there are many scenarios that aren't covered by a simple exclusion.

kpko commented 9 years ago

Hi,

is this still a thing? We have a huge typescript project and want to convert from the Visual Studio integrated typescript compilation to tsconfig.json files. We organized our files (mostly components) into many subfolders and it would be great if we could import them into the compilation without having to write hundreds of lines of "files" array items.

panuhorsmalahti commented 9 years ago

Tip: you can use the "unofficial" glob property with e.g. atom-typescript and it works fine (the plugin will autocomplete all the files from the globs). It would be nice to get this into core, of course.

awerlang commented 9 years ago

@kpko if you have a root folder from where you want to compile of files in subdirectories, just start tsc from that folder.

AFAIK, the current state doesn't handle well multiple input folders, and excluding unit and e2e tests inside feature folders.

philpowers commented 9 years ago

tsconfig.json absolutely needs glob support! There are just so many common use cases that are not covered by the very simple "exclude" support.

As a language that is supposed to make large-scale Javascript development more scalable and maintainable (and I do love TypeScript), it seems like being able to create a directory structure in whatever arbitrary format is most suitable to the project would be one of the most fundamental features. Currently this is NOT possible. Heck, even having basic wildcard support in both the "files" and "exclude" would be a big step in that direction.

tjoskar commented 9 years ago

@philpowers +1, I totally agree! We need glob support. However, typescript accepts pull request, right? So anyone who has a solution to the problem can make it real.

mhegazy commented 9 years ago

@rbuckton submitted a PR for this (https://github.com/Microsoft/TypeScript/pull/3232), but got busy with other issues. if someone is willing to take over the PR, and add verification for it, please let us know.

cjones26 commented 9 years ago

@panuhorsmalahti Great tip, thank you! Figured this would work but didn't actually try it until I saw your comment.

jimthedev commented 9 years ago

Since there hasn't been movement on this in a month I would just echo what others have noted.

An exclude property may accomplish some of the goals of globs but it does not go far enough. In fact I think it can give your team a false sense of security. It makes it too easy to break if you're working in a large team of devs of varying attention to detail. Someone drops a file in the root of the project and forgets to add it to the exclude? Bam you're in trouble. Team conventions are more often than not phrased like this: 'put all of your .ts files inside of ./app if you want them covered by code coverage'. Similarly, it would be helpful if typescript merged the filesGlob and files arrays for a really comprehensive solution to this problem.

nickroberts commented 9 years ago

Please listen to the community and add support for filesGlob. It makes things so much easier. We have people using all sorts of editors and IDEs, and it we were to have glob functionality, it would clean our code up so much more, along with providing a better cross platform solution.

We can achieve this through the npm package tsconfig-glob, but we really would like a standard solution, where we didn't have to run a gulp task.

atom-typescript already does a nice job of this, although all it does is update the files array (so does tsconfig-glob). If the files array could just accept globs...WIN!

I would gladly work on a solution for a PR, but, unfortunately, I haven't the time to spend working on this. :crying_cat_face:

mhegazy commented 9 years ago

This is on the list of features to support; that is why the issue is still open. The core team has limited resources, so we can not get to everything all the time. As mentioned in https://github.com/Microsoft/TypeScript/issues/1927#issuecomment-134720294, we are open to accepting help here.

krimple commented 9 years ago

+1 for filesGlob support on tsc... would be a great update.

zdychacek commented 9 years ago

+1 globbing should be standard feature

gentisaliu commented 9 years ago

+1

m4cx commented 9 years ago

+1

donaldpipowitch commented 9 years ago

Just want to point out that this sadly limits jsconfig.json, too. https://twitter.com/ErichGamma/status/653835875835604992

AbubakerB commented 9 years ago

+1

pleerock commented 9 years ago

+1

FlorianKoerner commented 9 years ago

+1

marcosbasualdo commented 9 years ago

+1

BretJohnson commented 9 years ago

+1 for filesGlob on tsc

hrajchert commented 9 years ago

+1