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

Declaring an extension to an ambient internal module inside of an external module #3282

Closed
oliverw opened this issue May 27, 2015 · 8 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@oliverw
Copy link

oliverw commented May 27, 2015

I'm currently migrating the WebRx codebase from a internal modules to external ones. WebRx consumes RxJS and implements one extension of the Observable class which is exported as an interface in the corresponding declaration file.

Now that my code-base consists of external modules I'm having a trouble to make this extension even accessible to my own external modules, let alone to consumers of the library.

Declaring the extension used to be a simple as this:

/// <reference path="../node_modules/rx/ts/rx.all.d.ts" />

declare module Rx {
    interface Observable<T> extends IObservable<T> {
        toProperty(initialValue?: T): wx.IObservableProperty<T>;
    }
}

Has now become this:

///<reference path="../node_modules/rx/ts/rx.all.d.ts" />
import { IObservableProperty } from "./Interfaces"

declare module Rx {
    export interface Observable<T> extends IObservable<T> {
        toProperty(initialValue?: T): IObservableProperty<T>;
    }
}

Which does not work. The compiler does not even recognize the existence of IObservable.

Any suggestions would be appreciated :)

@mhegazy
Copy link
Contributor

mhegazy commented May 27, 2015

As you noted you can not extend a module in the global namespace from within an external modules. external modules have their own scope.

there are two options, mainly depending on how you expect your users to reference the interface;

  1. declare the extension in a non-module global file, and add a /// reference to it from your .d.ts; this way you are telling the compiler that your .d.ts file depends on this other file that changes the global scope.
  2. define a new interface and let your users use it instead, this is more inline with ES6 modules, and would be the approach i would recommend, but it all depends on how you want consumption to look like..
import { IObservableProperty } from "./Interfaces"

// users will have to use the new interface, instead of Rx.IObservable directly
export interface WebRxIObservable<T> extends Rx.IObservable<T> {
    toProperty(initialValue?: T): IObservableProperty<T>;
}

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label May 27, 2015
@oliverw
Copy link
Author

oliverw commented May 27, 2015

@mhegazy Well that would not compile as well. The compiler claims not to know Rx-IObservable even though its defined in rx.all.d.ts

@oliverw
Copy link
Author

oliverw commented May 27, 2015

To provide some insight how toProperty is implemented:

///<reference path="../node_modules/rx/ts/rx.all.d.ts" />

import { IWebRxApp, IObservableProperty } from "./Interfaces"
import { args2Array, isFunction, isCommand, isRxObservable, throwError } from "./Core/Utils"
import IID from "./IID"
import { createScheduledSubject } from "./Core/ScheduledSubject"
import { Implements } from "./Core/Reflect"
import { injector } from "./Core/Injector"
import * as res from "./Core/Resources"

"use strict";

let RxObsConstructor = <any> Rx.Observable;   // this hack is neccessary because the .d.ts for RxJs declares Observable as an interface)

/**
* Creates an read-only observable property with an optional default value from the current (this) observable
* (Note: This is the equivalent to Knockout's ko.computed)
* @param {T} initialValue? Optional initial value, valid until the observable produces a value
*/
function toProperty(initialValue?: any, scheduler?: Rx.IScheduler) {
    scheduler = scheduler || Rx.Scheduler.currentThread;

    // initialize accessor function (read-only)
    let accessor: any = function propertyAccessor(newVal?: any): any {
        if (arguments.length > 0) {
            throwError("attempt to write to a read-only observable property");
        }

        if (accessor.sub == null) {
            accessor.sub = accessor._source.connect();
        }

        return accessor.value;
    };

    Implements(IID.IObservableProperty)(accessor);
    Implements(IID.IDisposable)(accessor);

    //////////////////////////////////
    // IDisposable implementation

    accessor.dispose = () => {
        if (accessor.sub) {
            accessor.sub.dispose();
            accessor.sub = null;
        }
    };

    //////////////////////////////////
    // IObservableProperty<T> implementation

    accessor.value = initialValue;

    // setup observables
    accessor.changedSubject = new Rx.Subject<any>();
    accessor.changed = accessor.changedSubject
        .publish()
        .refCount();

    accessor.changingSubject = new Rx.Subject<any>();
    accessor.changing = accessor.changingSubject
        .publish()
        .refCount();

    accessor.source = this;
    accessor.thrownExceptions = createScheduledSubject<Error>(scheduler, injector.get<IWebRxApp>(res.app).defaultExceptionHandler);

    //////////////////////////////////
    // implementation

    let firedInitial = false;

    accessor.sub = this
        .distinctUntilChanged()
        .subscribe(x => {
            // Suppress a non-change between initialValue and the first value
            // from a Subscribe
            if (firedInitial && x === accessor.value) {
                return;
            }

            firedInitial = true;

            accessor.changingSubject.onNext(x);
            accessor.value = x;
            accessor.changedSubject.onNext(x);
        }, x=> accessor.thrownExceptions.onNext(x));

    return accessor;
}

RxObsConstructor.prototype.toProperty = toProperty;

It's plain old prototypal inheritance. There must be a way to express this.

@mhegazy
Copy link
Contributor

mhegazy commented May 28, 2015

From this sample, this looks like you are extending the global Rx object, and for this your best bet is to have two files:

  1. rx.extenstions.d.ts
declare module Rx {
    interface Observable<T> extends IObservable<T> {
        toProperty(initialValue?: T): wx.IObservableProperty<T>;
    }
}

and 2. webrx.toproperty.ts

///<reference path="../node_modules/rx/ts/rx.all.d.ts" />
///<reference path="rx.extenstions.d.ts" />

import { IWebRxApp, IObservableProperty } from "./Interfaces"
....
let RxObsConstructor = <any> Rx.Observable;  

RxObsConstructor.prototype.toProperty = function toProperty(initialValue?: any, scheduler?: Rx.IScheduler) { ... };

your users would then just import "webrx.toproperty" and that will pull in the triple-slash reference to your other file, or if you have a hand-edited .d.ts, include the definitions in "rx.extenstions.d.ts"

Note that this is more or less what you had to do before since you are modifying the global object to add you extension. the other model i was referring to earlier is one that you wrap instead of append, which does not seem to work for Rx in its current form today, but for the sake of argument, assuming it does, you can do something like:

import { Observable } from "Rx"; // Assuming Rx.Observable is defined as a class
class WebrxObservable extends Observable {
    toProperty(initialValue?: any, scheduler?: Rx.IScheduler) {
        ...
    }
}
export {WebrxObservable  as Observable}

@oliverw
Copy link
Author

oliverw commented May 28, 2015

@mhegazy Unfortunately the approach you've outlined above won't work either because IObservableProperty is defined in the external module "./Interfaces". So we're back at square one because there does not seem to be a way to import IObservableProperty into rx.extenstions.d.ts. ;(

Regarding your class-level example:

Unfortunately Rx.Observable is defined as an interface. That's why I had to come up with that awkward approach in the first place. And it looks like I am not the only one struggling with that problem. Care to have a look?

@mhegazy
Copy link
Contributor

mhegazy commented May 28, 2015

Assuming interfaces.ts does not have any implementation code, but just interface/type definitions, an alternative is to define interfaces as such:

// wx.interfaces.d.ts
declare module wx {
    export interface IObservableProperty<T> extends Rx.IDisposable {
        (newValue: T): void;
        (): T;
        changing: Rx.Observable<T>;
        changed: Rx.Observable<T>;
        source?: Rx.Observable<T>;
    }
}

declare module "Interfaces" {
     export = wx;
}

you can then reference that the same way you do today:

/// <reference path="wx.interfaces.d.ts" />
import {IObservableProperty} from "Interfaces";

or as:

/// <reference path="wx.interfaces.d.ts" />
var property: wx.IObservableProperty;

@mhegazy
Copy link
Contributor

mhegazy commented May 28, 2015

As for RX defined as a class; the language does not allow for extending classes, so once you declare something as a class it is locked, the only way to extend it is subclassing. this is why type definitions for such frameworks are defined as interfaces to allow for extensions.

We are looking into 1.6 to enable ambient classes to merge with interfaces, so you can define RX as a class, and can still extend it with an interface as you do today.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2015

We are looking into enabling extending arbitrary expressions not just classes. this should allow you to extend from Rx without having to change the definition.

@mhegazy mhegazy closed this as completed Jun 13, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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

2 participants