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

Allow declaring that module is also function / constructor without declaration merging #5867

Closed
spion opened this issue Dec 1, 2015 · 5 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@spion
Copy link

spion commented Dec 1, 2015

Use case: express.d.ts on DefinitelyTyped:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/express/express.d.ts

Here, express exposes a namespace Express to let us extend Request via declaration merging.

Unfortunately those extensions cannot contain types imported from ambient external "modules", which means they're of limited use. You can't import {RedisClient} from 'redis' and define that such a client is attached to a Request (you're not allowed to import ambient external modules into a namespace declaration)

However, since express-the-module is also a function, the only way to declare the module is to use declaration merging then write export = mergeCombination at the end. This means that we can't use ambient external module merging either

Since lots of node modules love this "module is function and also namespace" approach, this isn't an isolated case.

I really think we need syntax that allows us to export functions and class constructors from module declarations together with everything else. Declaration merging with export = x syntax is cool, but it really is a hack when its used because of the lack of this feature.

Syntax (just to get started, I'm sure someone will have better ideas)

export new(x:string):MyType;
export (x:string):string;

// Generics

export new<T>(x:T):MyType<T>;
export <T>(x:T):T;
@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

ES6 modules do not allow for changing the module shape and/or exporting the module object as a function. the only way you would do this in ES6 is by exporting default. Modules are already hard enough for users to understand; and with default exports/imports, there is a variety of ways to define a module. I do not think we should be adding yet another way to express the shape of a module, specially that this style of modules is not part of the standard.

@spion
Copy link
Author

spion commented Dec 1, 2015

I don't propose this syntax to be valid outside of declarations.

Also, the facts are:

  • lots of CommonJS (node) modules use this
  • the declaration merging hack is the non-obvious, hard way to do it - having dedicated syntax would actually be easier for newbies compared to what we have now.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

we already have a concept of declaration merging, as well as export =. we can not remove these, or we will be breaking a lot of code out there. what i am saying is we should not add yet another way.

@mhegazy mhegazy added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision labels Dec 3, 2015
@mhegazy mhegazy closed this as completed Dec 3, 2015
@spion
Copy link
Author

spion commented Dec 4, 2015

I think you should reconsider this, as it remains the biggest pain point in writing definition files for node modules. After all, constructors and functions are already allowed in interfaces, and arbitrary restrictions to forbid them on modules are both nonsensical and harmful to usability.

As shown above, export = clearly does not compose well. Why not both make definitions easier to write and allow users to avoid the pitfalls of export = hack?

Writing definition files still feels like navigating a maze of gotchas. This change will greatly contribute towards reducing that problem.

@spion
Copy link
Author

spion commented Dec 4, 2015

Also note that I'm not saying to remove export = and break compatibility.

Furthermore I definitely think declaration merging should be kept as its useful in many more situations than the export = cases. For example, the original express.d.ts problem (the ability to attach e.g. a redis client to the request) could be fixed with declaration merging by simply

  1. in express.d.ts

    declare module 'express' { 
      interface Request {
        // ...
      }
      // ... other declarations
      export ():Application
    }
  2. defining additional declarations that will merge naturally with 'express', e.g. in express-redis-client-middleware.d.ts

    declare module 'express' {
      import {RedisClient} from 'redis'
      interface Request { redis: RedisClient; }
    }

There is no more reason to import redis into a namespace now (which is not allowed by the compiler anyway) and the namespace hack can simply be removed from express.d.ts. Now that export = isn't in use, merging with the ambient external module works fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants