DefinitelyTyped / tsd

[DEPRECATED] TypeScript Definition manager for DefinitelyTyped
Apache License 2.0
1.12k stars 135 forks source link

Support for scopes (eg. test, e2e) #147

Open gabor-farkas opened 9 years ago

gabor-farkas commented 9 years ago

In our project, we need different sets of dependences for the actual application, for karma unit tests and for protractor e2e test. What is the preferred way to do this? As a java developer I'd propose configurable dependency scopes, that generate different tsd.d.ts files, eg tsd-test.d.ts and tsd-e2e.d.ts. Another solution would be to allow the tsd.json definition file to be configurable and then we can handle this from our gulp script. What would be the preferred way to support different sets of dependencies?

blakeembrey commented 9 years ago

Hi @gabor-farkas. Can you give some more information on why you'd need/want multiple TSD file outputs instead of one? It seems overly complicated with no benefit to me, but I might be missing something.

gabor-farkas commented 9 years ago

Hi Blake, of course. Among our normal dependencies we have jquery.d.ts for example, but for e2e testing, we need protractor.d.ts which defines another $ named variable as cssSelectorHelper. So if I compile my application code together with my e2e testing code, typescript will give an error for that declaration.

With maven it was common that we have a different test scope (well, scopes are limited in maven unfortunately, so we couldn't have an even different e2e scope, we have to make a different module for that), and test scoped dependencies are not included for the application code compilation. Also, classes in test sources are not visible for the application code. So in my Java background this is the common way to solve this, but there might be a more elegant way in the js stack.

gsipos commented 9 years ago

This problem occurs to me too but with the angular.d.ts and angular-mocks.d.ts files conflict for example at inject methods declaration.

blakeembrey commented 9 years ago

@gabor-farkas Thanks! I understand the issue now. I'm not sure if this is something that can be simply solved with TSD. I see TSD as a dependency manager, but I don't see managing scopes within the project since it should be easy enough to write your own main d.ts file similar to tsd.d.ts. That said, there should be something to fix this situation. Creating "scopes" sounds like a hack and not a solution. I'd like to see modules not expose globals (since issues like this, by default these modules shouldn't be exporting globals) and allow the implementor to declare their own globals. Not sure if this can be enforced, but I'll look into it.

@gsipos I was looking at the repo in master and didn't see the conflict in the angularjs folder.

gabor-farkas commented 9 years ago

@blakeembrey well, I can of course write my own tsd.d.ts, but tsd could do that for me so that I don't make a mistake, I just have to declare my dependencies once in the json. Currently I created separate directories to handle the situation. If the tsd tool accepted an argument to override the default tsd.json declaration file, it would be a bit more flexible, but now I can actually live with this of course :) Thanks for the discussion!

blakeembrey commented 9 years ago

@gabor-farkas I'd have to look more into your setup to make sure I understand it all correctly, but wouldn't you need jQuery to still be defined even in your tests? We can't realistically have definitions with random conflicting globals our there. It will be possible to use multiple tsd.json files, but that will be based on the context of execution (E.g. run tsd in this folder). However, not a great a solution. I'm not dismissing the idea of scopes, just trying to avoid introducing something brittle that will introduce more issues down the line. Thanks for bringing up the global issue though, it hasn't fully occurred to me early as something I needed to factor into solving.

gabor-farkas commented 9 years ago

@blakeembrey we don't need jQuery in our e2e (protractor) tests, because they run inside node.js and they use selenium-webdriver to interact with the browser. jQuery would only work in the browser. That's why protractor defines it's own selectorhelper. (It's of course true that these type definitions should not aggressively define globals. Regardless to this, we should always keep our compilation source set as small as possible, avoiding unnecessary dependencies.)

pdfowler commented 9 years ago

I'll chime in with a separate, and related use case: My MEAN stack app, based on meanjs.org and using the pure vertical modules. In this sense, I have the problem of wanting Node, Express, etc defined for my server code, and Angular defined on my client code. Although this setup seems to be the exception, and not the rule for MEAN apps, I have found it to be extremely beneficial. My directory structure under my root modules/ directory can be summarized as follows:

I originally ran into a problem with my jshint config. It didn't feel right to add global defintions for it, should and other testing library specific methods. With the goal of keeping the global config clean, I found that you could place a .jshintrc at any level and it would apply to all files under it. Combine that functionality with nested inheritance of jshintrc and a grunt task to copy client, server or test specific .rc files to all /client/, /server/, */test/ directories and all was good in my jshint world - both in my build scripts and in my editor.

The problem I see now is one of auto-complete scopes. WebStorm, already a lumbering beast, has become unbearably slow as a result of having both client and server side auto-complete definitions loaded into the scope. There are ways to define scope on a per-directory basis, but this is an entirely manual process and I'm not willing to devote the hour to set it all up. This brought me to VSCode, which brought me here.

The two implementation options that immediately come to mind are:

  1. Support per-directory configuration files similar to JS Hint's. Inherit any typings from the root typings/tsd.d.ts
  2. Support multiple tsd.d.ts type files and define scope via tsd.json. For example, for all files matching */client/**, apply the root tsd.d.ts as well as the tsd-client.d.ts typings.

Hope that an explanation of my problem helps frame this discussion better from a different point of view and use case...

Diullei commented 9 years ago

We can use multiples tsd.json files using --config flag.

Example: tsd reinstall --config client/tsd.json andtsd reinstall --config server/tsd.json. Each file will define their own typing references and their own tsd.d.ts file.

What do you think?

blakeembrey commented 9 years ago

@Diullei You'd be better off leaving it to the user instead of increasing the size of the API. E.g. cd client && tsd reinstall or cd server && tsd reinstall.

gabor-farkas commented 9 years ago

@blakeembrey I think having a CLI argument for this makes it easier to include tsd in the build process.

Diullei commented 9 years ago

The current TSD version already supports the --config argument.

gabor-farkas commented 9 years ago

Ahh sorry i missed that. Thanks!

Diullei commented 9 years ago

Note: I fixed a bug with --config option. To get the latest version you must install tsd@0.6.5-beta

https://github.com/DefinitelyTyped/tsd/releases/tag/0.6.5-beta

blakeembrey commented 9 years ago

@gabor-farkas Unrelated, but why would you think that? Most build tool processed make use of a CWD and in the case of using the programmatic API, the CWD would be configured.

gabor-farkas commented 9 years ago

@blakeembrey that's true. It must be due to my ant-maven background that I tend to consider cwd "cheating" :)