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

Module/class augmentation doesn't work #11406

Closed
MoonStorm opened this issue Oct 5, 2016 · 10 comments
Closed

Module/class augmentation doesn't work #11406

MoonStorm opened this issue Oct 5, 2016 · 10 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@MoonStorm
Copy link

MoonStorm commented Oct 5, 2016

TypeScript Version: 2.0.3

Code

// A *self-contained* demonstration of the problem follows...
// rxjs installed via npm
// seems to expose everything via its .d.ts file:
export { Observable } from './Observable';

// attempting to augment the Observable class exposed by the rxjs module
// observable.operators.ts
import {Observable} from 'rxjs'

declare module 'rxjs' {
    interface Observable<T> {
        doStuff():void;
    }
}

// consumer.ts
import { Observable, Subscription } from 'rxjs'
export class Consumer{
  private _sub:Subscription;
   constructor(){
      this._sub = Observable.from([1,2,3]).subscribe();
   }
}

Expected behavior:
The doStuff to appear available in the rxjs module on the Observable class.

Actual behavior:
Attempting to use import {Observable} from 'rxjs' in another file (consumer.ts) causes compilation errors.

Export declaration conflicts with exported declaration of 'Observable'
Property 'subscribe' does not exist on type 'Observable'
etc.

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Oct 5, 2016
@RyanCavanaugh
Copy link
Member

I don't see any calls to doStuff in your repro code. Please don't make us guess.

@MoonStorm
Copy link
Author

MoonStorm commented Oct 5, 2016

More info added in the description of the problem.

The doStuff doesn't matter. Ironically enough, a similar example using Observable was posted in the TS guidebook. In that sample, the augmentation is done at the same level as the Observable.d.ts file with import {Observable} from './Observable', as if we're gonna augment modules straight in the their folder, inside node_modules!?

@RyanCavanaugh
Copy link
Member

The correct declaration looks like this:

import {Observable} from 'rxjs'

declare module 'rxjs/Observable' {
    interface Observable<T> {
        doStuff():void;
    }
}
import { Observable, Subscription } from 'rxjs'
export class Consumer{
  private _sub:Subscription;
   constructor(){
      this._sub = Observable.from([1,2,3]).subscribe();
      Observable.from([1, 2]).doStuff(); // OK
   }
}

@RyanCavanaugh RyanCavanaugh added Question An issue which isn't directly actionable in code and removed Needs More Info The issue still hasn't been fully clarified labels Oct 5, 2016
@MoonStorm
Copy link
Author

MoonStorm commented Oct 5, 2016

Yep, that works. Thanks.
Could you please update the docs with this example, in the module augmentation section as well as in what's new ? Nobody is going to augment rxjs's Observable straight in the external module's own folder.

@RyanCavanaugh
Copy link
Member

The example is just supposed to be an example and it doesn't mention rxjs at all. Do we need to use class names that no one has ever used before?

@MoonStorm
Copy link
Author

MoonStorm commented Oct 5, 2016

Haha, no of course not. Just saying you've picked a name of a very popular and extendable class of a very well known library. I bet I'm not going to be the only one being thrown off by that example.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 5, 2016

my bad. that example was actually modeled after RXJS code a la: https://github.com/ReactiveX/rxjs/blob/master/src/add/operator/map.ts. in hind sight, probably not the best example for the general public.

@MoonStorm
Copy link
Author

MoonStorm commented Oct 5, 2016

No worries. It would probably be much useful to show how to augment an external module, may that be ''rxjs'' or a fictive one.

I had never seen a module declaration like that. The augmentation seems inconsistent with the way module externals are being used regularly. Is that because the ''Observable'' is being re-exported by the lib's main module? Aren't we tying ourselves up with the inner structure of the external module?

@RyanCavanaugh
Copy link
Member

Is that because the ''Observable'' is being re-exported by the lib's main module? Aren't we tying ourselves up with the inner structure of the external module?

Correct. It's tricky because it's not apparent that augmenting an alias should effect the underlying thing (what if it was aliased under a completely different name?). But this is how the runtime generally has to operate as well for augmentation -- if you're not setting on the correct root object, it may not work.

@MoonStorm
Copy link
Author

I undestand now. Obviously not a vey sustainable solution long term. The runtime could be helped through a keyword acting as a hint that an augmentation is at play, which means the root needs to be resolved and changed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants