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

Unable to do module augmentation (1.8 beta) #6722

Closed
bytenik opened this issue Jan 29, 2016 · 10 comments · Fixed by #6742
Closed

Unable to do module augmentation (1.8 beta) #6722

bytenik opened this issue Jan 29, 2016 · 10 comments · Fixed by #6742
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@bytenik
Copy link

bytenik commented Jan 29, 2016

import { Request } from 'express';

declare module 'express' {
    interface Request {
        id: number;        
    }    
}

The above code sits alone in a file that I then import from various locations.

The code fails to compile, warning Module augmentation cannot introduce new names in the top level scope.

@ahejlsberg
Copy link
Member

The problem is that express.d.ts uses an export = declaration so there isn't actually a top-level interface Request in the module. Ideally what needs to happen is that when a module uses export =, an augmentation for that module should augment the entity named in the export =.

@ahejlsberg ahejlsberg added the Bug A bug in TypeScript label Jan 29, 2016
@bytenik
Copy link
Author

bytenik commented Jan 29, 2016

For now, is there a hack to work around this (other than supplying my own version of Express's type definition file)?

@vladima vladima added this to the TypeScript 1.8.2 milestone Jan 29, 2016
@vladima
Copy link
Contributor

vladima commented Jan 29, 2016

@ahejlsberg just to clarify: do we want to allow augmentation for export = entities only if they have namespace meaning (which is more in line with our existing behavior i.e. for doing import * as ns from "...").

@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

Why not augment the global declaration that export= is exporting instead using

declare global {
    namespace Express {
        interface Request {
          ......
        }
    }
 }
`

@ahejlsberg
Copy link
Member

Yes, we should do it only if the export = entity has a namespace meaning.

On Jan 29, 2016, at 8:02 AM, Vladimir Matveev <notifications@github.commailto:notifications@github.com> wrote:

@ahejlsberghttps://github.com/ahejlsberg just to clarify: do we want to allow augmentation for export = entities only if they have namespace meaning (which is more in line with our existing behavior).

Reply to this email directly or view it on GitHubhttps://github.com//issues/6722#issuecomment-176837450.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

Yes, we should do it only if the export = entity has a namespace meaning.

This entity might also be visible from outside the module. augmenting it, would be unexpected, i would argue. most of the definitely typed export= are using a global namespace as the source of the export.

I think our position of modules with export = are not extendable is a valid one, and we should not change it.

@vladima vladima closed this as completed Jan 29, 2016
@vladima vladima reopened this Jan 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

had a discussion with @vladima offline. I think i see how this works conceptually now. export= makes the module an alias to the exported declaration, and changing either would change the other.

@weswigham
Copy link
Member

Also, @mhegazy - express doesn't use a global Express module object in its definition (though it does define one which exposes some empty interfaces you can append to in order to extend certain things, like Request in the given example) - it does this:

declare module "express" {
  function e(): e.Express;
  module e {
    //...
  }

  export = e;
}

which, without being able to merge into the export='d symbol, there's no way to go in and modify it.

@bytenik
Copy link
Author

bytenik commented Jan 29, 2016

Luckily in my specific use case, the express module's Request object implements the Express namespace's Request interface, which lets me use a global augmentation. This resolves this one particular issue this one time, but I'm sure this is going to come up often.

@raybooysen
Copy link

Hi @vladima
With your PR, how do we augment entities exported via export =? It doesn't seem clear

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants