Proposal for correct version lookup in deeply embedded non-TS external modules #4673
Comments
|
I take it this problem is supposed to arise when In this specific example, you change your TS library ( In effect, I'm saying to stop using Additionally, this makes typings discoverable on |
|
That's an interesting idea. A big disadvantage I see with it, though, is that I'm then dependent on the provider of that typed package to provide high-quality typings in the first place, and then update it every time the source package changes. I'm afraid that we'll have tens of these typed-* packages (for one source package), just like we have tens of webpack-typescript, gulp-typescript, etc etc packages. On DT, this is much less of a problem because 'anyone' can contribute, and there's many people to approve the changes.
Yeah, but that's in my proposal even simpler: as soon as the real package has the typing, it will automatically be used over the DT one, no need to change anything :) |
|
If you'd like, you can just restructure the DT org (or make another similar one) to publish npm packages in the new style, and accept contributions in kind. The contribution model is not contingent on the distribution one. |
Clarification: I meant to say 'per package', there's tens of same-but-different packages. My fear is that the same will happen for basically every other package. The bigger ones will likely converge into a group of people maintaining them, but for the smaller ones... What could work, maybe, is if we can automatically generate npm packages out of the DT repo. That would have the best of both worlds. Hmmm :) |
|
@weswigham Haha, my thoughts exactly :) |
There's 4 different dts for node on DT already (0.8, 0.11, etc). DT already has this issue (and larger versioning issues). By using |
|
@poelstra Yes. It should be possible to autogenerate external declaration dts files from DT types. I've been going through and doing it by hand for node.d.ts, though. Plus, not everything on DT is a node package. |
|
It would be ideal if we could somehow add ES6 module information to the lookup process. For instance, if --target es6 is specified, then look for utils.es6.d.ts first before falling back to utils.d.ts. Or perhaps based on the style of the import statement. ES6-style import statement looks for es6.d.ts, CJS-style import statement does not. |
|
@jbrantly Would an ES6 version of the package really be different from a non-ES6 typing in terms of how the end-user can use it? Can you give an example of such a difference? Because I think that if the typing simply follows the style of the package itself (either ES6 exports or the 'old' way), the user of the library can already choose to use old or new style imports, and the compiler will do the 'conversion' for you. So it would only really matter if you want your typing to work on e.g. TS 1.4 or lower, but I'm not sure that's worth the trouble. |
|
@poelstra Absolutely. The difference is in module assignment (not available in ES6) and default exports (not available in CJS). Here are some examples: These two concepts are fundamentally incompatible (you can't both use |
|
Addendum:
If you're using |
|
Hmyeah, ran into the But that was in fact a case of 'trying to use an ES5 export for an ES6 module', which is why I added the
Well, I can always import it using
Yes, but as explained in e.g. #2719 (comment), Babel uses a trick both when exporting and importing a package. TS now also applies the same trick when exporting (such that e.g. Babel can use a TS package as-is in both modes), but it doesn't apply the necessary check when importing (as far as I know). The former case is irrelevant here (because Babel doesn't use these typings), and the latter case means that even if we'd provide both
You can always |
While true, it's not always "convenient" (debatable, I know).
Which is why I suggested the --target es6 check. You're right that TS doesn't do the dynamic check if you're targeting ES5, but it's certainly possible to target ES6 and then use Babel or another transpiler to go to ES5/CJS which does do the dynamic check. FWIW, I think your point does cement that it would need to be based on --target and not the import style.
If an ambient module declaration uses As an aside, it could also (possibly) be useful for built-in things like |
Interesting point. But this may lead to the situation where the compiler emits ES6 code without complaints, which will only work when you're indeed going to pull them through Babel. If you'd use a non-dynamic-check-loader, it will fail at runtime.
But simply 'automagically re-typing' it as something else (if it really does
What do you mean? I think both 'proper external' is equally applicable to CJS and ES6?
I think our discussion shows that there are valid use-cases for it, but that it should not be the default behaviour :)
I've been thinking about that recently as well, but I came to the conclusion that an ES5/CJS package should not provide such definitions if it doesn't also provide the implementation. I.e. I think it would be a good thing that the compiler tells me "unknown type Promise", such that I know I need to provide a polyfill myself when I'm using Node 0.10 (and that polyfill package might in turn make the Promise type available globally). Or that if I'm compiling for 0.12, it will already work without complaints (even if I'm just using --target es5). Seems the TS team is already working on something to facilitate the latter in #4168, btw. |
I think you misunderstood what I was saying. Say you've written a module in ES6 and used Babel to compile it. You want to provide typings, so you write a proper external module file for it ( export = 'a';Now a consumer using TypeScript comes along and is using import * as someModule from 'someModule';As it stands today they will get an error if they tried to consume the module: That is what I meant when I said we need the feature.
Agreed, but note that what I'm saying is that would be left up to the definition file authors, not TypeScript. TypeScript would just be providing a way to load an ES6 definition. It's entirely possible that the ES6 definition simply omits a default definition in this case forcing a |
|
So you can reproduce what I'm saying: // test.d.ts
interface MyTestInterface {
test: string;
}
export default MyTestInterface;
// app.ts
import * as test from 'test';Run tsc using Now change |
|
So with my better understanding of what you meant by "Don't expose any declared modules to global module namespace" I feel like we could kind of merge this and #4668. What would you think of:
For now this does not address the "global" keyword concept. Pretty much all globals are hidden except for those directly referenced by your application. Like I said, I personally think this is sufficient but would be open to adding another way for deeply nested modules to somehow declare globals that are not hidden. Thoughts? |
|
Re number 2, looking back I'm not sure we actually discussed isolating globals in the context of proper external modules anywhere, but I still hope I can convince you that it is needed (for the same reason it's needed for mixed-mode modules). In other words: // wtf.d.ts
declare var SomeGlobal: any;
// myutils.d.ts (a proper external module)
/// <reference path="wtf.d.ts" />
export = 'a'There's nothing stopping someone from writing that, for example. I think we'd still want to isolate globals in that case. |
|
Thanks for the feedback! I don't have the time to address this right now, but I'll look at it later. |
|
@jbrantly Regarding the es6 detection: ok, I'm not into that "--target es6" stuff (yet), so I believe you :) I've updated the proposal with some of your points, but I don't agree with all of them yet. Again, not so much time, I'll explain later. |
|
@poelstra Thanks for incorporating some of my points. To reinforce the point that proper external modules should also have the same global isolation logic applied I've created some concrete examples that show how not having global isolation can result in issues. You should be able to pull/run these yourself to play with them if you desire. https://github.com/jbrantly/TS4668_promise Both of these follow this sentence from your current proposal: "Note that if an isomorphic typing includes a 'proper external' typing, and the proper external typing Please feel free to point out any errors or misconceptions in my examples. Also, I could very well be putting words into @weswigham's mouth (forgive me and correct me if I am), but I believe he might feel along similar veins based on #4694 (even if the details are slightly different, the basic idea is that it's nice for definitions to be able to create globals, but not for those globals to leak too far up the chain). |
|
https://github.com/jbrantly/TS4668_promise captures the essence of the issue I bring up in #4694 perfectly, and illustrates how globals should not be leaked to parent packages unless the parent package has explicitly opted in to receive them, yea. That it's possible is arguably an oversight in how |
Me too! @weswigham You're absolutely right about not wanting to leak things like However, note that in my proposal, I think this is actually handled gracefully:
In 'proper external' typings, people can't accidentally create globals, unless they directly In the 'non-proper' variants, my 'mixed mode' will apply, so stuff will be isolated. This does assume that people need to be a bit careful when they start writing proper external typings: any use of |
|
@jbrantly Thanks for your example repos! I think our ideas aren't actually very far from eachother :) In your promise example repo, So, what you're basically saying here, is: this package window.Promise = Es6Promise;In a way, this is why I sort-of coined the (It's a bit tricky to use the Perfect! So, if I, anywhere in my program, e.g. in Where I see things go wrong in your proposal, is when you state that you want to be able to 'opt-in' to making the globals available again. Because how do you know that when you import a package, that global is actually 'the' global that's going to be available at runtime? Let's modify your example a bit to illustrate this point. Suppose that I create a package Let Now, with your proposal, in Of course, new is always better :), so I decide to reference // app.ts
import "new-promise";
var p = new Promise();
p.then(...).done();No complaints from the compiler in your case, because the In my proposal, the So, if I'd only been using Now, we probably would then also need some kind of 'override' to tell the compiler that we manually made sure that e.g. I'm pretty certain that globals that lead to 'real' globals in JS-land (variables, classes, functions) should equally be made available as globals in the typings. Maybe this is less so for things like interfaces, type aliases, etc. Btw, note that I think this is completely different from the case you mention in #4337, where Same for a 'normal' version of the Whoa, this became way longer than I hoped. Thanks if you stayed with me this long ;) |
|
@poelstra No worries on the long text, sometimes that's what it takes to get our ideas across.
So to summarize (I think we both already knew this), we essentially agree that the example is a problem but we disagree on how to go about resolving it. I feel like hiding everything and then manually opting-in to globals is best because it puts the user in control and guards better against accidental leaks. It also (I think) makes it easier to write isomorphic typings (more on this later). You feel that exposing and somehow merging the globals is best because it allows the type system to show potential issues with the "one global wins" scenario.
This is a crucial part of the spec in your head but not the spec in the OP 😀 You should add this to your proposal, otherwise your proposal as-is results in duplicate identifier errors as I showed in my example.
Yea, this is the part that I think would be a PITA.
I think we should address this. In this case, what actually is the way, if you want to use the polyfilled global, to do so? I would assume first off that the es6-promise definition wouldn't include the global, and would instead put that in an un-referenced definition file. So two things: 1) how does that definition file access the Promise typings in the first place (see isomorphic discussion below, same problem, really) and 2) how does a user reference that definition file to pull in the global? I think something like
Yes and no. If you look at that proposal it uses this: // mylib-module.d.ts
/// <reference path="mylib-common.d.ts" />
declare module 'mylib' {
export = __MyLib;
}But the whole point of this is to use proper external modules, right? So rewriting: // mylib-module.d.ts
/// <reference path="mylib-common.d.ts" />
export = __MyLib;And now __MyLib is available globally (in your proposal). Now you might say why use __MyLib here? The right way would be to write this: // mylib-module.d.ts
export var foo: string;
export var bar: string;OK, but now what about the namespace version that I also need to write. There is no way to somehow take the proper external module and throw it into a namespace (#2018). So I would need to duplicate the definitions. // mylib-namespace.d.ts
declare namespace MyLib {
export var foo: string;
export var bar: string;
}This is no good, and what #4337 is all about. Note that if TS did have better facilities to handle this it wouldn't be an issue (and I think TS definitely needs better facilities for this). |
Yes, good summary :)
You're right, oops! This spec originally wasn't really about mixed mode, but about how to find the files. Guess its focus has shifted... Added it now!
Added to discussion items.
I was thinking about wrapping it in a dedicated file/package for that:: // polyfill-promise.ts
<reference path="something-that-declares-Promise.d.ts" />
import { Promise } from "es6-promise";
Promise.polyfill();
// or e.g.: global.Promise = Promise;Now everytime you import this package/file, you'll have the global
The former yes, the latter might not be necessary, see below.
That's tricky indeed. It would look like // declare-promise.d.ts (a 'proper external' typing)
import { Promise } from "es6-promise";
export global Promise;
// or e.g.: global var Promise = Promise;
// or e.g.: global.Promise = Promise;Not sure what the syntax inside e.g.
Answered above: simply import as usual.
Haven't tried, but I think it does. It's certainly possible using ambient declarations (
Nah, not sure. For modules that are only intended to work in Node or ES6, sure. It'd be nice if the following would work, though: // my-module-isomorphic.d.ts
declare module "my-module" {
export * from "my-module"; // trying to reference `my-module.d.ts` here
}That doesn't work though, because of the circular reference, and because that export doesn't re-export default.
Yup, but in this case you would indeed 'violate' the 'proper external best-practice' of not using
Fully agree, and I hope our discussions help to shape them. |
mhegazy
added
the
@types
|
The conclusion here was to use npm for distributing declaration files, and use the npm dependency model to resolve conflicts. Declaration files need to be modules, and not global. for UMD modules they should be using the new Please see more documentation about authoring declaration files and distributing them on npm at https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/declaration%20files/Introduction.md |
Update 20150908: applied feedback from comments. Thanks @jbrantly! Also renamed "Legacy mode" to "Mixed mode" and clarified what it's about.
Update 20150909: more feedback from @jbrantly.
Now that #2338 is implemented, we have nice resolution of typings for 'native TS modules'.
However, exporting typings for non-TS modules can be problematic in case of deeper dependency trees.
Issue #2839 (by me) contains a proposal to solve this, but does not talk about deeper-than-one dependencies in case of conflicting versions. Issue #4665 (by @mhegazy) also addresses non-TS resolution, but does not explicitly mention the lookup logic.
Example dependency tree
Assumptions about this tree:
Problem description
In this case, because
mylibis the 'first' package that knows about TS, it somehow needs to provide the typings for all non-TS stuff it exposes:foolib@1.0,barlib@1.0but also bothutils@3.0andutils@4.0.In an ideal world, using 'proper external modules' (see #4665 and #2338) would solve this if they were all TS modules (i.e. they provide their own typings in their npm package).
In this case though, the
importstatements need to resolve to e.g. DefinitelyTyped typings, typically located in e.g.mylib/typings/.So,
mylib/typings/needs to have autils.d.tsfor version 3.0 and 4.0, and the compiler needs to know how to find them, and which version to use.Especially note the difference between the concept of the current JS module versus current TS module in the following proposal.
Proposed algorithm
Naming (taken from #4665):
declare module "Y" { ... }declarationsdeclare module "Y" { ... }, but directly export their classes, variables, etc.When compiling a package
X, and looking for typings of an external moduleY/Z, let:CurrentTSModule= XCurrentJSModule= XZ="index"if module is imported without a path (i.e.import "Y"instead ofimport "Y/Z")Now:
Y'spackage.json (starting atCurrentJSModule`)node_modulesof parent packagesYprovides its own typing (eitherindex.d.tsor by followingtypingsproperty inpackage.jsonin the package directory)CurrentTSModuleandCurrentJSModulevariables toY, i.e. any external modules used byYshould be resolved by looking in<Y>/node_modulesand<Y>/typings/, no longer in<X>/typings/YinX's typings folder (let's call ittypings/):typings/<Y>@<majorY.minorY.patchY>/<Z>.d.ts(proper mode)typings/<Y>@<majorY.minorY>/<Z>.d.ts(proper mode)typings/<Y>@<majorY>/<Z>.d.ts(proper mode)typings/<Y>/<Z>.d.ts(proper mode)typings/<Y>/<Y>.d.ts(mixed mode)typings/<Y>.d.ts(mixed mode)<library ... />tags<library ... />tag, matchY'spackage.jsonname and version against the name and semver specification in the<library>tagCurrentTSModuleasX, but setCurrentJSModuletoY<reference>'d declarations, to the global module namespace, anddeclare module "Y/Z" { ... }and use its contents as the result (i.e. 'convert to proper external')Notes:
<majorY>etc. are based on the version as found inY'spackage.jsonnode_modules, lookups intypingsdo not traversetypings/dirs of parent packages. Only those of (TS-)packageXare searched.foolibusesutils@3.0, butbarlibusesutils@4.0(i.e.CurrentTSModulestaysmylib, butCurrentJSModuleswitches fromfoolibtobarlib)Mixed mode
Mixed mode (previously called "legacy mode"), is intended to allow existing DefinitelyTyped typings
(which use 'ambient external' scheme) to basically be used as external modules (CommonJS) without making a lot of 'accidental' globals available (e.g. the
Promisetype inbluebirdtypings), while still also allowing them to be used in AMD and plain script modes ('isomorphic typings').These isomorphic typings typically declare lots of things as globals (perfect for plain script mode),
but these are usually not made globally available when loaded as CommonJS.
So, the idea is to wrap the whole typing into its own private space, then only make the actually requested external module part of it available.
Note that if an isomorphic typing includes a 'proper external' typing, and the proper external typing
<reference>'s another typing, that typing is still allowed to declare globals (they will not be 'isolated').Having two packages declare the same global (e.g. when a proper external typing references a .d.ts, which explicitly marks a variable, class, etc as being globally available) currently leads to a compiler error ("Duplicate identifier").
One idea I had was to 'merge' the types of such globals instead (e.g. if two packages both declare a
Promise, but they are in fact of different types, the resulting global will be typed as e.g.declare var Promise: PromiseType1|PromiseType2;This way, the compiler can error when the global is actually used (as opposed to when declared) in an incompatible way (see #4673 (comment))..
Discussion items
Algorithm A has the advantage of being 'filesystem based', just like node's environment.
But B supports more complex semver matches and prevents bikeshedding over e.g. the name and structure of
typings/.typings/<Y>.es6.d.tsovertypings/<Y>.d.tswhen available. Or possiblytypings/<Y>.<target>.d.tswhere<target>is the--targetpassed to tsc. See comments below for discussions on pros/cons.The text was updated successfully, but these errors were encountered: