Circular reference between Observable and others

[kwonoj]

Observable.ts

...
import {ConnectableObservable} from './observable/ConnectableObservable';
import {Subject} from './Subject'; --> import subject

...
if (observerOrNext instanceof Subscriber || observerOrNext instanceof Subject) {

Subject.ts

...
import {Observer} from './Observer';
import {Observable} from './Observable'; --> import observable
...
export class Subject<T> extends Observable<T>
...

ends up with below snippet

import {Observable} from 'dist/cjs/Observable';

Observable.of(1).subscribe(console.log);

 d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
                                                                   ^
TypeError: Cannot read property 'prototype' of undefined
    at __extends (D:\github\RxJS\dist\cjs\Subject.js:4:68)
    at D:\github\RxJS\dist\cjs\Subject.js:20:5
    at Object.<anonymous> (D:\github\RxJS\dist\cjs\Subject.js:135:3)

Subject does not resolves reference to Observable.

[kwonoj]

Not sure if #894 is related or not..

[benlesh]

It's probably #894. TypeScript is pretty good about hoisting dependencies that would otherwise be circular.

import {Observable} from 'dist/cjs/Observable'; will just get you the bare-bones Observable without from and of right now.

[kwonoj]

Did couple of try, seems #894 and this need to be updated both. even import from / of to observable, importing observable directly would not work due to this.

[kwonoj]

Changed subject to describe this more general, such as

• Observable <--> GroupedObservable
• Observable <--> CoreOperators <--> GroupedObservable ..(direction is not precise, just an example)

and lot of others.

This reference could possibly cause runtime reference resolve issue as described above and also noticeably increases build times.

Tried to solve this via simple ambient declaration but wasn't work as expected, ambient merging seems possible way but still need to prove.

[ntilwalli]

@kwonoj Just a quick comment, dunno if it's relevant, I introduced this circular import with #863 and to deal with it I updated the KitchenSink and Rx files to move the Subject import before Observable. I made a comment to that effect in the discussion and in the checkin notes. Until I reordered these imports, I was seeing the same error.

[kwonoj]

@ntilwalli I assume top-level (Rx / Kitchensink) is also somewhat related, but not critical at this moment. This issue is specific for module-level import (i.e, import observable only, or something else) to make reference resolve failure to work with specific module. Once this is cleanly resolved, may able to visit top level module also using similar approaches.

[benlesh]

@jeffbcross ran a circular dependency check on #912 and found it was green. So that PR is likely all we need to clear this issue.

Also, we're going to add some sort of circular dependency checking to the test build. (again, @jeffbcross)

[jeffbcross]

PR for ci check: #917

[kwonoj]

Cool news, then assume #909 can be close without merging? @Blesh

[benlesh]

#912 closes this.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.