-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
External module resolution for non-Typescript packages #2839
Comments
My hacky suggestion:
In this case again if we only read
We only read the following files into the compilation context of Note in:
Compilation context simply picks the latest versions, so : HackyNote that if you are using the browser (single version of each js) then this is probably what you want. |
@basarat Thanks for looking into this! 🌹 In my mail I saw a different post than the one above, seems you've editted it :) Anyway, you made a valid point, so let me comment on that:
I may have phrased it a little unclear. I agree that Re your 'hacky solution': I don't think a conflict resolution like you're proposing is going to work, and may lead to uncompilable code. Example of it going wrong using 'wrapped' external modules: /// mylib/typings/myutils.d.ts (i.e. the 1.0 typings)
declare module "myutils" {
export interface SomeOldInterface { ... }
export declare function something(): SomeOldInterface;
}
/// mylib/dist/mylib.d.ts
declare module "mylib" {
import myutils = require("myutils");
export declare function myfunc(): myutils.SomeOldInterface;
}
/// myotherlib/typings/myutils.d.ts (i.e. the 2.0 typings)
declare module "myutils" {
export interface SomeNewerInterface { ... }
export declare function something(): SomeNewerInterface;
}
/// myotherlib/dist/myotherlib.d.ts
declare module "myotherlib" {
import myutils = require("myutils");
export declare function myotherfunc(): myutils.SomeNewInterface;
} I didn't actually try to compile this, but I'm sure we'll get a compiler error on the return value of either Contrast this with the following case: /// mylib/typings/myutils.d.ts
export interface SomeOldInterface { ... }
export declare function something(): SomeOldInterface;
/// mylib/dist/mylib.d.ts
import myutils = require("myutils");
export declare function myfunc(): myutils.SomeOldInterface;
/// myotherlib/typings/myutils.d.ts
export interface SomeNewerInterface { ... }
export declare function something(): SomeNewerInterface;
/// myotherlib/dist/myotherlib.d.ts
import myutils = require("myutils");
export declare function myotherfunc(): myutils.SomeNewInterface; Because external module resolution in |
many times :) |
Yes my hacky solution would fail miserably for that. |
I've made your example concrete: https://github.com/basarat/typescript-node/tree/master/poelstra1/ts Here is the directory tree : If you open |
@basarat You are (again) amazing 👍 🌹 Note though, that I think the structure you have created now resembles #2338 more than it does this issue (because This is issue is more about the case when If you like, I can update your repo with two other folders (maybe clone them to poelstra2 and 3, to allow easy comparison):
|
Alright, I went ahead, and created those two branches in basarat/typescript-node@22d703a Obviously, they don't work yet.
As I had to hand-edit the files/structure and have no way of verifying, there may be e.g. a typo in a path here and there, but I think it conveys the general idea pretty well. |
Can you extend the example (which I think is a good idea) to cover the case where the types come from DefinitelyTyped that has not yet been modified as you suggest? One of the "features" of DefinitelyTyped today is the ability to have a single d.ts file that contains the declarations for many modules. The prime example is node.d.ts. How would that be handled? Not all of the types are referenced with the appropriate module scoping. For example, in bluebird.d.ts, you can reference it and thus gain access to the Promise class. You can then use that type in your own d.ts file. I suppose the "right" thing to do would be to require the Bluebird module to be imported, and then only refer to the Promise class via the imported name. I like the theory behind restructuring DefinitelyTyped as outlined here, but I wonder how long it would take to accomplish. I sometimes wish for a parallel repository like npm where we can simply publish the d.ts files by claiming the namespace for individual modules, rather than centrally managing it like the DefinitelyTyped repo. Alternately, I want to be able to point to one or more alternate typings repos. If your proposal would allow extensibility in the list of repos, then that would allow new-style repos to be developed piecemeal. |
@mhfrantz You make some very interesting remarks!
Maybe, for the sake of reducing the scope of this issue, I shouldn't even have mentioned the suggestion to restructure DT. It might be that it happens automatically anyway, once #2338 gets traction. |
Concerning the |
@mhfrantz I just pushed basarat/typescript-node@467b8b0, which includes an unmodified Bluebird typing in Additionally, it contains
To test, simply fire up Atom Typescript, add |
@mhfrantz Applied the suggestion you had to rename the bluebird import from Interesting to note that the hover over the |
Wouldn't this be solved by having #2338 support multiple typing definition files (whatever they end up being called)?
When TypeScript is parsing This would solve the problem of packages shipping with their dependencies included inside of them. |
There is, of course, another scenario where your dependency manager doesn't nest like this (to limit duplication). Take this scenario (imagine all the same file contents as above except for the tsconfig.json files):
If left like that, all of the libraries would resolve
I believe this is a more realistic (and more ideal) example than the previous one, but it requires more tooling buy-in. Personally, I think tooling buy-in will come naturally once you give a well defined way to solve this sort of problem. TSD, JSPM, etc. can all easily generate the appropriate configuration files when installing dependencies. In fact, JSPM already creates a couple for similar reasons. |
@Zoltu Thanks for chiming in!
Do I understand you correctly, that you're suggesting to use a lookup in a If so, I think there's still two different cases:
For 1, I think there's no need to explicitly specify anything, it should automatically work through the resolution magic in #2338. That would ensure that the correct typings version is always chosen, instead of having to remember to update |
I have a feeling that you're trying to 'redirect' all requests for a certain version of a certain package to one single .d.ts file, correct? So if two TS packages both use e.g. Bluebird (or at least the same major+minor of it), that there's really just 1 .d.ts file referenced? I've been thinking about that before, but I imagined this could work by creating symlinks (even on Windows). So it would simply work with 'normal' files, but a tool like In that case, that tool could also automatically deduce the version by looking at e.g. Note that de-duplication is only an 'optimization', not a strict requirement, because e.g. the same interface definition in two different d.ts files will be equivalent. We used to do some bower in the past, but now switched everything to npm, so I may very well be overlooking important use-cases that you may have. |
I apologize for not being clear, I don't think that a single d.ts should be used for different versions of a package. What I am proposing is that keeping typings and JS libraries in sync is not the job for TSC, it is the job for package managers or other tooling. The same goes for keeping What I envision is the following:
In the above example, you are downloading a The same steps would be followed if you downloaded a package that came with its own definitions. The only thing that would change is that the package would likely not contain the In the case of nested dependencies, see what I have above but remember that any definition package in the dependency tree can depend on a regular JavaScript (no definitions) package that contains the JS for which the definition package defines. If you liked that one, you may also like:
|
@poelstra To directly reply to your first question: I do think that there should be an explicit mapping for everything. While auto-mapping is nice for hello world applications and small scripts, I would much rather have explicit control over how things get mapped. That being said, I would definitely not want to manually fill out these mappings by hand all the time. I believe tooling can easily solve this problem though. In modern languages we find it insane to manually manage dependencies (at least we should) like back in the C/C++ days. The same should be true about manually managing project configuration files. This is part of what makes NuGet for C# so great, the tooling does all of the heavy lifting "magic" to make things "just work" but the compiler doesn't need to know anything about where the packages came from. TL;DR: The compiler should not contain auto-resolution logic or magic file resolution. If someone wants these things tooling can be built separate from the compiler such as into the package manager. |
That means jquery is installed 'below' jquery-typescript-definitions, so I still can't
That requires some very TS-specific changes to npm, doesn't it? I think it's 'easier' to get TS to adapt to NPM than NPM to TS.
That's a very good point, and I initially thought this would be a good idea too. However, we're then depending on the author of jquery-typescript-definitions to keep up-to-date on newer versions of jquery. In practice, though, after a year of TS development we've become a bit reluctant to this type of dependency (e.g. gulp-* packages lagging behind the package they're wrapping). And we've also noticed that a package's typings generally keep working surprisingly well, as long as the major version doesn't change.
I tend to agree, having less automagic is always nice. But I don't really see why a list of 'package name -> filename' in a One of the main reasons why I prefer to use the filesystem instead of an entry in tsconfig.json, is that tsconfig.json needs to be in version control (for the compiler and formatting options), and having tooling automatically update it is causing a lot of unnecessary merge conflicts etc. So, in my vision, your example would then go like:
Benefits:
|
Having things "just work" by allowing the user to run with mismatched js/d.ts is nice right up until you have a problem, then it is all of a sudden a huge problem instead of a simple one. On numerous occasions I have wished Visual Studio didn't require exact matches for PDB files and source code. However, at least when I don't have an exact match I know what the problem is and the steps to fix it (either change my PDB or change my DLL). If Visual Studio made it all "just work" instead I would get incredibly bizarre debugging errors and oddities. The same is true for the nightmare that is C/C++ dependency management. You get a DLL/LIB and a header file. It will let you compile with a mismatch, but then you get sometimes incredibly difficult to debug problems. If instead it just told you at build time, "your stuff doesn't match" you have a relatively easy problem to solve.
Yes. I was using NPM in this example but I acknowledge that NPM changes for TypeScript support are unlikely unless TypeScript gets huge adoption. More likely though are other package managers like JSPM, which IMO is almost perfect for the browser development world. Alternatively, people could write wrappers around NPM that solve this problem or they could author pre-compile tools that do the work to map NPM-retrieved definitions to the correct NPM-retrieved libraries. In your example,
This is a very good point and a flaw in my most recent proposal, primarily due to the way NPM works (this is not a problem in JSPM for example). I suppose this would require revision to the proposal to support mapping the compiled code as well. An example structure off the top of my head:
The above would result in the definitions being found and also the compiler replacing the call to I will be honest, I am not a node developer so I don't use NPM or CommonJS for my dependency manager or module loader. In the browser world, I believe JSPM/SystemJS to be the future of dependency management and module loading, and having a mapping file fits in nicely there. I think part of the problem here is that TypeScript wants to work in two different worlds with two different paradigms. One of those worlds (NPM/NodeJS/CommonJS) are legacy systems that are stuck with choices made many years ago, without the benefit of hindsight. My goal with my comments here and on other issues is to come up with a solution that is agnostic to the underlying dependency manager and module loader while being flexible enough to work with all of the existing systems. I personally believe that the compiler should not depend on files being in a certain place on the file system (such as a typings folder). Instead, the developer should explicitly pass things into the compiler that the compiler needs, or provide them through a compiler config file ( |
Sorry for the delay, but we finally have the node resolution in place, and i believe we need to put this proposal into effect soon to avoid creating a reference hell for node packages. I have a proposal in #4665, based on the issues and ideas in this issue, and would love to get your feedback. |
I just created a "Typescript NPM Package" and I get the following error: Exported external package typings file cannot contain tripleslash references. My package uses express, compression, mongoose and other npm packages. If I remove my |
@lucasmciruzzi At the moment you need to document to the user that they need these external to your project There are workarounds under discussion to handle such packages and packages that abuse (use) the global namespace |
Thanks @basarat! I'll keep an eye on this topics to update my package as soon as the solution is defined :) |
This should be resolved with the move to Please see my comment in #4673 (comment) for more details. also see: |
Good progress is made on supporting the case of packages that carry their own typings in #2338.
That proposal doesn't currently deal with modules that do not carry their own typings, a.k.a. all modules that need typings from DefinitelyTyped.
This issue is to investigate possibilities of, and propose a (simple) solution for preventing problems when using (multiple versions of) external modules together.
TL;DR
declare module "..." { }
in typings for external modulesExample scenario
Suppose that both
mylib
andmyotherlib
export a function, that returns a value of a type defined inmyutils
.Existing scenario
Likely, both
mylib
andmyotherlib
will each have a/// <reference path="./typings/tsd.d.ts" />
line, which again has a/// <reference path="./myutils/myutils.d.ts" />
line.Problem
Both libs carry their own version of
myutils.d.ts
, which works fine as long as they are not used together.However, when they are used together (as in this example), all the
/// <reference ...>
lines will (need to) be 'merged', which essentially means thedeclare module "myutils" { ... }
from bothmylib
andmyotherlib
will end up in the same 'global module name namespace'.This gives a compiler error, and even if the compiler would somehow allow it, would make it impossible to refer to either version of
myutils
.Additionally, tools like
dts-generator
e.g. include the/// <reference>
line inside of thedeclare
blocks, which get ignored by the compiler. This leads to unknown-references when trying to use such a typing, and will typically be solved by adding e.g. amyutils/myutils.d.ts
tomyprogram/typings/tsd.d.ts
, which will be wrong for one of the libs.Solution
External modules have the nice property that there is no 'global namespace' (except the filesystem, maybe), and basically every reference is 'local'.
It makes sense to use the same referencing scheme that's used for resolving 'local' external modules (within a package) for 'normal' external modules (different npm packages).
Basically, the idea consists of:
declare module "..." { }
in typings for external modules (to prevent the global namespace clash)typings/
folder)Advantages
/// <reference>
lines are no longer needed in most cases (except probably for e.g. importing Mocha'sit()
and other 'true' globals)declare module "..." { }
bytsc
, so all 'sorts' of external module typings will look the sametsd.d.ts
-way will still work for packages that do still usedeclare module "..." { }
(although it will still have the name-clash problem etc.)Disadvantages
Example 'new-style' typings
Most hand-crafted typings will typically look like:
But just to make things more interesting, suppose the typings of
myutils
look like:Nothing special here, this is what
tsc
generates today.(Note: dist/ may be because someone used CoffeeScript, not TypeScript, right? :))
Now, the typings dir on e.g. DefinitelyTyped could look like:
(Or maybe
2.0
instead oflatest
, ideas welcome.)And both
mylib
andmyotherlib
would have:Looking up .d.ts
Given e.g.
import { something } from "myutils"
the compiler could usemyutils/package.json
'smain
property to get to./dist/foo.js
, which in turn would resolve todist/foo.d.ts
.Nice and simple: no need to have support from the authors of
myutils
, no/// <reference>
lines.And it even supports the
import bar = require("myutils/dist/bar")
case.Open questions
declare
'ed, e.g. through tsd.d.ts? If so, would probably make first bullet usable, nice for gradual upgrade path of DT typings.myutils/latest/index.d.ts
will probably work out-of-the-box with external module resolution logic, butmyutils/latest/myutils.d.ts
or evenmyutils/myutils.d.ts
may be easier for filename in editor tab?Good idea? Bad idea?
The text was updated successfully, but these errors were encountered: