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

1.4: d.ts definitions can only import ambient external modules in other ambient modules #2198

Closed
jasonswearingen opened this issue Mar 4, 2015 · 21 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@jasonswearingen
Copy link

I am writing a d.ts for hapi, and wish to specify that some types used by the API are nodejs types.

For example:

server.route({handler:(request,reply)=>{
//can pass a nodejs stream to reply
 }});

however, as nodejs's stream.Stream is declared inside an ambient module, I can not simply import stream = require("stream"); at the top of my hapi.d.ts file. Doing so causes the error: Error 278 Ambient external modules cannot be nested in other modules.

I am forced to define hapi inside of an ambient module also:

declare module "hapi" {
    import stream = require("stream");
        ///hapi definitions here....
}

an unfortunate side effect of this limition is that all module definitions that depend on ambient modules must also be defined in ambient modules. Instead, I would prefer defining modules in the following way:

declare module Hapi{
 ///hapi definitions here
}
declare module "hapi"{
 export = Hapi;
}

I prefer this methodology as it allows discovery/use of the type definitions without importing the ambient module.

Please consider allowing some workaround for this issue.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Mar 24, 2015
@3nsoft
Copy link

3nsoft commented Apr 16, 2015

I join @mhegazy in desire to have above way of module definitions.
The reason for this is automation of creating declaration files. I have node module. gulp-typescript creates definitions. Now, making declarations that are good for DefinitelyTyped, requires manual spaghetti-shuffling.
Please, please, please. Either let above solution happen, or provide a scriptable (!) work-around, which can properly added into modern build process.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 16, 2015

@3nsoft, can you elaborate on your scenario and the proposed solution?

You would want a single .d.ts file for all your node modules? or a definition file that works for both modules and injecting into the global namespace?

@3nsoft
Copy link

3nsoft commented Apr 16, 2015

@mhegazy here is my scenario.

For my node library, which is made of many files, I can have an auto-magic gulp task, which creates definitions (.d.ts's) for all files. And definition for main script looks as follows (actual output):

/**
 * This file is an external interface of Ecma-NaCl library.
 */
export import secret_box = require('./boxes/secret_box');
export import box = require('./boxes/box');
export import nonce = require('./util/nonce');
export import signing = require('./signing/sign');
export import fileXSP = require('./file/xsp');
import sha512Mod = require('./hash/sha512');
export declare module hashing.sha512 {
    var hash: typeof sha512Mod.hash;
    var makeHasher: typeof sha512Mod.makeHasher;
}
import scryptMod = require('./scrypt/scrypt');
export declare var scrypt: typeof scryptMod.scrypt;
export import arrays = require('./util/arrays');
export declare function compareVectors(x: any, y: any): boolean;

When someone npm install's the library, they need a definition like:

declare module "ecma-nacl" {
...
}

The simplest thing in a build environment would be to put all exposed definitions (one file only) into this declaration "bracket". Internally, result is a bunch of definition files requiring each other. Externally, it look like a folder with files, which can be copy-pasted into lib-user's declarations folder. And it can be easily send to DefinitelyTyped.

Can this happen, say, in a future version?
If not, what is an automatable work-around to generate ambient module declarations out of declarations that are generated on a classical node lib (commonjs external module)?
It does not matter if solution is a single or a multi-file. What matters is that it is automatic.

For further reference, here is a gulp task that does compilation, and creation of sources:

gulp.task('lib', function() {
    var ts = gulp.src([ SRC+'/lib/**/*.ts', SRC+'/typings/**/*.ts' ])
    .pipe(typescript({
        target: 'ES5',
        module: 'commonjs',
        declarationFiles: true
    }));
    return merge(ts.js.pipe(gulp.dest(DIST+'/lib')),
            ts.dts.pipe(gulp.dest(DIST+'/lib-declarations')));
});

May be there is a magic with typescript parameters, which will let me define ambient modules. So far I see no work-around to automate declarations part of a build process.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 16, 2015

thanks @3nsoft. I think your request should be better covered by #2568. It is something we will be looking to add in 1.6.

@3nsoft
Copy link

3nsoft commented Apr 16, 2015

@mhegazy Which is a link for this feature request, where I should vote, sign a petition, etc. :)

On a serious note, till 1.6, is there a non-manual work around?

@3nsoft
Copy link

3nsoft commented Apr 16, 2015

@mhegazy I am browsing referenced issues, and an emphasis there is about single file declarations. May be it is an internal view on how thing should be done.
My emphasis, As a lib developer with typescript, is "will it help me automate a build process of usable declarations".
The message is, help me automate this task. The solution may have one or many steps -- you, guys, have lots of room for internal design, but the task should be automatable.

@jasonswearingen
Copy link
Author

I find it extremely constraining to force anything that depends on nodejs's definition to be an ambient module.

can you just "allow" import of ambient modules? (don't raise Error 278 Ambient external modules cannot be nested in other modules.) does that screw with namespacing?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 16, 2015

the problem here is it does not really model reality. if i use your internal module typings that import http,

module app {
    import http = require("http");
    function createServer() :http.Server;
}

The compiler will not compain if i use it in a web application including the file in a script tag as:

var x = app.createServer();

This is clearly not going to work for multiple reasons. 1. I am not on node, and http does not exist, and 2. I am not using a model loader (i.e. calling require), so even if i am in node, nothing works.

This is the main difference between internal and external modules. external modules have a concept of dependencies and an expectation of a loader that manages them; and i do not see how you can impose that on internal modules, which are basically a variable pointing to an object literal.

@jasonswearingen
Copy link
Author

Reading my original issue post, I did not make one point clear: the typings found in node.d.tsare ambient purely for aesthetic reasons. the typings could have been defined like this, with absolutely no change in behavior:

declare module Stream{
 ///nodejs stream module definitions here
}
declare module "stream"{
 export = Stream;
}

if the node.d.ts typings were done this way it would allow me to also construct definitions in a similar way. however because the definitions are done in this way:

declare module "stream"{
 ///nodejs stream module definitions here
}

I am forced to write definitions that use stream typings as ambient definitions.

@3nsoft
Copy link

3nsoft commented Apr 17, 2015

@mhegazy which reality are you talking about? For example, in my very-very common reality, gulp automatically (a) compiles all typescript files, and (b) browserify-es every node dependency into single package for browser's build. It all works, when everything is properly packed, and your example is incorrect, because browserify creates require utility with all packages for consumption in a browser.

What is missing, though, it is a 100% automatic generation of declarations. Currently declarations need manual intervention due to restrictions on mixing modules.

As you said, initial design of TypeScript has, quote,

main difference between internal and external modules. external modules have a concept of
dependencies and an expectation of a loader that manages them,

It happens to assume a certain loading. Loading can be done differently in javascript for the same business logic code. Most node packages loading for browsers is taken care by proper packaging with browserify. Let TypeScript work for other assumptions in loading.

When package is used in node, all dependencies are added to package.json, and npm automatically sets up environment, so that all required code is available. Again, your example is incorrect, as npm install fixes the worries you expressed.
And again, if there is no declaration, users cannot enjoy types. But making declarations is not automatable due to restriction on mixing modules in declarations.

I, personally, love typescript. Switched in last half year all my projects to it. The only pain. Real pain. Annoying pain. It is declarations.
And the funny thing is not that I have a problem without such declarations. I do not, because I have my lib's ts files in my folder. But when I use my lib the way other developer will, I notice that declarations are required.
In other words, if you want TypeScript to spread like fire, make creation of declarations automatable.

Put it another way. Do you want me to quietly write TypeScript, compile it, and tell the wider world that I give it a JavaScript library. Or, do you want me to tell the wider world that it is TypeScript library, here are declarations -- enjoy. What do you want me to tell others?

Again, it really does not matter how this is fixed / accomplished. On a first glance it looks like removal of restriction on module mixing will allow full build automation (among which is automation of declaration writing). But, if there is or should be another way of automating declarations, tell us, show it to us, or implement a work around, and tell us about it, so that we can spread TypeScript's glory further with our own works.

@3nsoft
Copy link

3nsoft commented Apr 17, 2015

@jasonswearingen first part of you post is not showing on the web, but is showing in the email. I'll echo it here:

@mhegazy I think you are conflating two issues:

  1. my issue (defining d.ts non-ambient definitions that use ambient definitions)

  2. using definitions without the proper modules loaded.

ECHO END

@jasonswearingen
Copy link
Author

I removed it because I decided it didn't really add anything constructive to the conversation.

@3nsoft
Copy link

3nsoft commented Apr 17, 2015

I think the guy did conflated issues, and out of this confusion is not able
to see our pain.

2015-04-16 22:30 GMT-04:00 Jason Swearingen [email protected]:

I removed it because I decided it didn't really add anything constructive
to the conversation.


Reply to this email directly or view it on GitHub
#2198 (comment)
.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 17, 2015

@3nsoft i believe i understand your request. and it is different from the original suggestion @jasonswearingen is proposing. and #2568 is a more appropriate place for your issue.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 17, 2015

the typings found in node.d.tsare ambient purely for aesthetic reasons. the typings could have been defined like this, with absolutely no change in behavior:

it is true that types do not exist at run-time, but they model something that exist. when you define your module as an external module, you expect it to be loaded using its name, using a module loader. if you define it as an internal module you are defining it as a variable in the global namespace, that can be accessed as such. Allowing this specific pattern would make it easier for your scenario, but would break others.

TypeScript's type system is structural; so you can copy your type definitions from node, and put them in internal modules, or wrap them in another module; which is something that I had to do personally for tsserver.
I understand the convenience aspect, and the need to maintain two sets of typings, and the need for a better solution; this is why i have marked the issue as a "suggession". I do not agree with the proposed solution though.

@jasonswearingen
Copy link
Author

@mhegazy you are correct that the object may or may not exist at runtime. But we are discussing types, not instances of objects. Types are not things, they are abstract "ideas".

I wish to make typings available for reuse without needing to construct the actual container (module) at runtime. This is actually possible for me to do, unless one of my dependencies has an ambient definition In that case I am stuck, and can not reuse the types freely. Thus my request.

@jasonswearingen
Copy link
Author

I should mention: Typescript types being abstract ideas. (not true in most other languages)

@jasonswearingen
Copy link
Author

here is a practical example where the current design fails:

in PhantomJs, to use the lodash external module you need to do the following:

var _:_.LoDashStatic = require("./lodash");

if the makers of the lodash.d.ts would have put the definition in an ambient module, the typings would NOT be usable in PhantomJs, as there would be no way for me to access them! (PhantomJs does not allow ambient external modules)

Thankfully the lodash.d.ts makers did it the "right way".

@felicienfrancois
Copy link

I'm facing the same issue.

My env is both nodejs and browser (https://github.com/atom/electron)
In my app, I don't use the import extmodule = require("extmodule") syntax to load external modules but the var extmodule = require("extmodule") one.
Here some reasons for this choices:

  • To fasten my app startup, I load some modules asynchronously, when I need it, which seems not to be possible with the import syntax
  • My app is splitted is several parts including a common part that manage dependency loading. As a result, I need Typings in parts where I don't load dependencies.

As a result, and because of the design of node.d.ts, I can't get typings for all external module not defined as ambient.
I need an ambient definition, so I can define the type of the return of require:

interface NodeRequireFunction {
    (id: 'modulename'): TypeOfModule
}

Bad design ?

More widely, I think this is a bad design to link dependency loading with typings, these are 2 different things that are NOT related.
Typings could be used (by an app or third party module) without loading the module itself (for another implementation of a standard lib for example).

The ability to import an ambient module into another ambient module declaration is not a good solution,

  • because it is misleading (same syntax as runtime module loading, but it does not load the module, only definitions, isn't it?)
  • because it is not possible to use without loading a module (either the original one or the nested declared one)

What can be done

Update node.d.ts definition (and also all others)

@mhegazy is right in a way, it could be a breaking change to attribute the same name to non ambient modules, not because they may not exists at runtime but, because it may exists something else with the same name (events, stream, ... are common names) in the global namespace.
The solution is to keep the design of node.d.ts, which already define global interfaces (that does not exists at runtime) under the NodeJS namespace.
It would be:

declare module NodeJS {
    module ModuleName {
     /// definitions here
    }
}
declare module "modulename" {
 export = NodeJS.ModuleName;
}
interface NodeRequireFunction {
    (id: 'modulename'): typeof NodeJS.ModuleName
}

But this is a bit off topic as most type def are maintained on https://github.com/DefinitelyTyped/DefinitelyTyped
If I catch the time to, I may post a few pull request to it

Allow import of typings from any context

And maybe with a different syntax than the module import syntax

@mhegazy mhegazy added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Dec 10, 2015
@cspotcode
Copy link

@felicienfrancois

  • To fasten my app startup, I load some modules asynchronously, when I need it, which seems not to be possible with the import syntax
  • My app is splitted is several parts including a common part that manage dependency loading. As a result, I need Typings in parts where I don't load dependencies.

I believe these issues can be handled using the pattern described here: http://www.typescriptlang.org/Handbook#modules-optional-module-loading-and-other-advanced-loading-scenarios
If you import a module and use it solely for types, it doesn't generate a require() call.

@jasonswearingen
Copy link
Author

old, will reopen if i run into this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants