Skip to content

Improve from #1528

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@
"copy_src_cjs": "mkdirp ./dist/cjs/src && cp -r ./src/* ./dist/cjs/src",
"copy_src_es6": "mkdirp ./dist/es6/src && cp -r ./src/* ./dist/es6/src",
"commit": "git-cz",
"compile_dist_amd": "tsc typings/main/ambient/es6-shim/index.d.ts ./dist/amd/src/Rx.ts ./dist/amd/src/Rx.KitchenSink.ts ./dist/amd/src/Rx.DOM.ts ./dist/amd/src/add/observable/of.ts -m amd --sourceMap --outDir ./dist/amd --target ES5 --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"compile_dist_cjs": "tsc typings/main/ambient/es6-shim/index.d.ts ./dist/cjs/src/Rx.ts ./dist/cjs/src/Rx.KitchenSink.ts ./dist/cjs/src/Rx.DOM.ts ./dist/cjs/src/add/observable/of.ts -m commonjs --sourceMap --outDir ./dist/cjs --target ES5 -d --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"compile_dist_es6": "tsc ./dist/es6/src/Rx.ts ./dist/es6/src/Rx.KitchenSink.ts ./dist/es6/src/Rx.DOM.ts ./dist/es6/src/add/observable/of.ts -m es2015 --sourceMap --outDir ./dist/es6 --target ES6 -d --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"compile_dist_amd": "tsc typings/main/ambient/es6-shim/index.d.ts ./spec/es5.d.ts ./dist/amd/src/Rx.ts ./dist/amd/src/Rx.KitchenSink.ts ./dist/amd/src/Rx.DOM.ts ./dist/amd/src/add/observable/of.ts -m amd --sourceMap --outDir ./dist/amd --target ES5 --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"compile_dist_cjs": "tsc typings/main/ambient/es6-shim/index.d.ts ./spec/es5.d.ts ./dist/cjs/src/Rx.ts ./dist/cjs/src/Rx.KitchenSink.ts ./dist/cjs/src/Rx.DOM.ts ./dist/cjs/src/add/observable/of.ts -m commonjs --sourceMap --outDir ./dist/cjs --target ES5 -d --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"compile_dist_es6": "tsc ./dist/es6/src/Rx.ts ./dist/es6/src/Rx.KitchenSink.ts ./dist/es6/src/Rx.DOM.ts ./dist/es6/src/add/observable/of.ts -m es2015 --sourceMap --outDir ./dist/es6 --target ES6 -d --diagnostics --pretty --noImplicitAny --suppressImplicitAnyIndexErrors",
"cover": "npm-run-all cover_test cover_remapping",
"cover_test": "rm -rf dist/cjs && tsc typings/main/ambient/es6-shim/index.d.ts src/Rx.ts src/Rx.KitchenSink.ts src/Rx.DOM.ts src/add/observable/of.ts -m commonjs --outDir dist/cjs --sourceMap --target ES5 -d && istanbul cover -x \"spec-js/**/*\" ./node_modules/mocha/bin/_mocha -- --opts spec/support/default.opts spec-js",
"cover_remapping": "remap-istanbul -i coverage/coverage.json -o coverage/coverage-remapped.json && remap-istanbul -i coverage/coverage.json -o coverage/coverage-remapped.lcov -t lcovonly && remap-istanbul -i coverage/coverage.json -o coverage/coverage-remapped -t html",
Expand Down
13 changes: 13 additions & 0 deletions spec/es5.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
interface Symbol { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this type definition's placed under test cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically because specs will fail to compile without it, and there is no way to indicate in tsconfig.json to add one additional file, without declaring all the files. The easiest place was to add it in the specs folder, and then reference it from the builds.

interface SymbolConstructor {
iterator: symbol;
}
declare var Symbol: SymbolConstructor;
interface Iterable<T> {
[Symbol.iterator](): Iterator<T>;
}
interface Iterator<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's difference this to what we're currently using in es6-shim type definitions? looks identical.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

es6-shim doesn't actually shim in Symbol, so the IteratorShim<T> doesn't have the appropriate [Symbol.iterator]() attached to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant was Iterator specific. Sorry for confusion.

next(value?: any): IteratorResult<T>;
return?(value?: any): IteratorResult<T>;
throw?(e?: any): IteratorResult<T>;
}
36 changes: 28 additions & 8 deletions spec/observables/from-spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {expect} from 'chai';
import * as Rx from '../../dist/cjs/Rx.KitchenSink';
import {$$iterator} from '../../dist/cjs/symbol/iterator';
import {Observablesque} from '../../dist/cjs/Observable';

declare const {expectObservable, Symbol};
declare const {expectObservable, Symbol, type};
declare const rxTestScheduler: Rx.TestScheduler;
const Observable = Rx.Observable;

Expand All @@ -26,7 +27,7 @@ describe('Observable.from', () => {
it('should handle an ArrayLike', function (done: MochaDone) {
this.timeout(300);

const arrayLike = {
const arrayLike: ArrayLike<number> = {
length: 3,
0: 1,
1: 2,
Expand Down Expand Up @@ -64,7 +65,7 @@ describe('Observable.from', () => {
it('should handle an ArrayLike with a mapFn', function (done: MochaDone) {
this.timeout(300);

const arrayLike = {
const arrayLike: ArrayLike<number> = {
length: 3,
0: 1,
1: 2,
Expand All @@ -83,7 +84,7 @@ describe('Observable.from', () => {
});

it('should handle an ArrayLike with a thisArg', (done: MochaDone) => {
const arrayLike = {
const arrayLike: ArrayLike<number> = {
length: 3,
0: 1,
1: 2,
Expand Down Expand Up @@ -116,7 +117,7 @@ describe('Observable.from', () => {
});

it('should handle an "observableque" object', (done: MochaDone) => {
const observablesque = {};
const observablesque: Observablesque<any> = <any>{};

observablesque[Symbol.observable] = () => {
return {
Expand All @@ -137,7 +138,7 @@ describe('Observable.from', () => {
});

it('should accept scheduler for observableque object', () => {
const observablesque = {};
const observablesque: Observablesque<any> = <any>{};

observablesque[Symbol.observable] = () => {
return {
Expand Down Expand Up @@ -166,7 +167,7 @@ describe('Observable.from', () => {
});

it('should handle any iterable thing', (done: MochaDone) => {
const iterable = {};
const iterable: Iterable<string> = <any>{};
const iteratorResults = [
{ value: 'one', done: false },
{ value: 'two', done: false },
Expand Down Expand Up @@ -195,7 +196,7 @@ describe('Observable.from', () => {

it('should throw for non observable object', () => {
const r = () => {
Observable.from({}).subscribe();
Observable.from(<any>{}).subscribe();
};

expect(r).to.throw();
Expand All @@ -212,4 +213,23 @@ describe('Observable.from', () => {
done();
});
});

it('should return T for ObservableLike objects', () => {
type(() => {
/* tslint:disable:no-unused-variable */
let o1: Rx.Observable<number> = Observable.from(<number[]>[], Rx.Scheduler.asap);
let o2: Rx.Observable<string> = Observable.from(<Iterable<string>>{});
let o3: Rx.Observable<{ a: string }> = Observable.from(Observable.empty<{ a: string }>());
let o4: Rx.Observable<{ b: number }> = Observable.from(new Promise<{b: number}>(resolve => resolve()));
/* tslint:enable:no-unused-variable */
});
});

it('should return T and map for arrays', () => {
type(() => {
/* tslint:disable:no-unused-variable */
let o1: Rx.Observable<number> = Observable.from(<number[]>[], x => x.toString(), null, Rx.Scheduler.asap);
/* tslint:enable:no-unused-variable */
});
});
});
4 changes: 2 additions & 2 deletions spec/observables/zip-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('Observable.zip', () => {

describe('with iterables', () => {
it('should zip them with values', () => {
const myIterator = {
const myIterator: Iterable<number> = <any>{
count: 0,
next: function () {
return { value: this.count++, done: false };
Expand All @@ -103,7 +103,7 @@ describe('Observable.zip', () => {

it('should only call `next` as needed', () => {
let nextCalled = 0;
const myIterator = {
const myIterator: Iterable<number> = <any>{
count: 0,
next: () => {
nextCalled++;
Expand Down
6 changes: 3 additions & 3 deletions spec/operators/mergeMap-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Observable.prototype.mergeMap', () => {
it('should map values to constant rejected promises and merge', (done: MochaDone) => {
const source = Rx.Observable.from([4, 3, 2, 1]);
const project = function (value) {
return Observable.from(Promise.reject(42));
return Observable.from(Promise.reject<number>(42));
};

source.mergeMap(project).subscribe(
Expand Down Expand Up @@ -82,11 +82,11 @@ describe('Observable.prototype.mergeMap', () => {
it('should map values to rejected promises and merge', (done: MochaDone) => {
const source = Rx.Observable.from([4, 3, 2, 1]);
const project = function (value, index) {
return Observable.from(Promise.reject('' + value + '-' + index));
return Observable.from(Promise.reject<string>('' + value + '-' + index));
};

source.mergeMap(project).subscribe(
(x: number) => {
(x: string) => {
done(new Error('Subscriber next handler not supposed to be called.'));
},
(err: any) => {
Expand Down
4 changes: 2 additions & 2 deletions spec/operators/zip-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('Observable.prototype.zip', () => {

describe('with iterables', () => {
it('should zip them with values', () => {
const myIterator = {
const myIterator: Iterable<number> = <any>{
count: 0,
next: function () {
return { value: this.count++, done: false };
Expand All @@ -102,7 +102,7 @@ describe('Observable.prototype.zip', () => {

it('should only call `next` as needed', () => {
let nextCalled = 0;
const myIterator = {
const myIterator: Iterable<number> = <any>{
count: 0,
next: function () {
nextCalled++;
Expand Down
10 changes: 7 additions & 3 deletions src/Observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ export interface Subscribable<T> {
subscribe(observer: Observer<T>): AnonymousSubscription;
}

export type SubscribableOrPromise<T> = Subscribable<T> | Promise<T>;
export type ArrayOrIterator<T> = Iterator<T> | ArrayLike<T>;
export type ObservableInput<T> = SubscribableOrPromise<T> | ArrayOrIterator<T>;
export interface Observablesque<T> {
[Symbol.observable](): Subscribable<T>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing here. [Symbol.observable]() should return a type that is Subscribable<T> AND Observablesque<T> at a minimum. (Similar to how Iterators have a Symbol.iterator on them)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what? Scratch that. Don't worry about it, for the purposes of the internals of our library where this is being used, we don't care.

}

export type SubscribableOrPromise<T> = Observablesque<T> | Subscribable<T> | Promise<T>;
export type ArrayOrIterable<T> = Iterable<T> | ArrayLike<T>;
export type ObservableInput<T> = SubscribableOrPromise<T> | ArrayOrIterable<T>;

/**
* A representation of any set of values over any amount of time. This the most basic building block
Expand Down
9 changes: 8 additions & 1 deletion src/observable/ArrayObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ export class ArrayObservable<T> extends Observable<T> {
* @name of
* @owner Observable
*/
static of<T>(...array: Array<T | Scheduler>): Observable<T> {
static of<T>(item1: T, scheduler?: Scheduler): Observable<T>;
static of<T>(item1: T, item2: T, scheduler?: Scheduler): Observable<T>;
static of<T>(item1: T, item2: T, item3: T, scheduler?: Scheduler): Observable<T>;
static of<T>(item1: T, item2: T, item3: T, item4: T, scheduler?: Scheduler): Observable<T>;
static of<T>(item1: T, item2: T, item3: T, item4: T, item5: T, scheduler?: Scheduler): Observable<T>;
static of<T>(item1: T, item2: T, item3: T, item4: T, item5: T, item6: T, scheduler?: Scheduler): Observable<T>;
static of<T>(...array: Array<T | Scheduler>): Observable<T>;
static of<T>(...array: Array<T | Scheduler>): Observable<T> {
let scheduler = <Scheduler>array[array.length - 1];
if (isScheduler(scheduler)) {
array.pop();
Expand Down
7 changes: 6 additions & 1 deletion src/observable/FromObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ export class FromObservable<T> extends Observable<T> {
* @name from
* @owner Observable
*/
static create<T>(ish: any, mapFnOrScheduler?: Scheduler | ((x: any, y: number) => T), thisArg?: any, lastScheduler?: Scheduler): Observable<T> {
static create<T>(ish: ObservableInput<T>, scheduler?: Scheduler): Observable<T>;
static create<T, R>(ish: ArrayLike<T>, mapFn: (x: any, y: number) => R, thisArg?: any, scheduler?: Scheduler): Observable<R>;
static create<T>(ish: ObservableInput<T>,
mapFnOrScheduler?: Scheduler | ((x: any, y: number) => T),
thisArg?: any,
lastScheduler?: Scheduler): Observable<T> {
let scheduler: Scheduler = null;
let mapFn: (x: any, i: number) => T = null;
if (isFunction(mapFnOrScheduler)) {
Expand Down
7 changes: 4 additions & 3 deletions src/symbol/observable.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/// <reference path="./symbol.d.ts" />
import {root} from '../util/root';

const Symbol: any = root.Symbol;
const Symbol = root.Symbol;

export let $$observable: any;
export let $$observable: symbol;

if (typeof Symbol === 'function') {
if (Symbol.observable) {
Expand All @@ -16,5 +17,5 @@ if (typeof Symbol === 'function') {
Symbol.observable = $$observable;
}
} else {
$$observable = '@@observable';
$$observable = <any>'@@observable';
}
3 changes: 3 additions & 0 deletions src/symbol/symbol.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
declare interface SymbolConstructor {
observable: symbol;
}
20 changes: 12 additions & 8 deletions src/util/subscribeToResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {root} from './root';
import {isArray} from './isArray';
import {isPromise} from './isPromise';
import {Subscriber} from '../Subscriber';
import {Observable} from '../Observable';
import {Observable, ObservableInput} from '../Observable';
import {$$iterator} from '../symbol/iterator';
import {$$observable} from '../symbol/observable';
import {Subscription} from '../Subscription';
Expand All @@ -12,16 +12,20 @@ import {OuterSubscriber} from '../OuterSubscriber';
export function subscribeToResult<T, R>(outerSubscriber: OuterSubscriber<T, R>,
result: any,
outerValue?: T,
outerIndex?: number): Subscription {
let destination: Subscriber<R> = new InnerSubscriber(outerSubscriber, outerValue, outerIndex);
outerIndex?: number): Subscription;
export function subscribeToResult<T>(outerSubscriber: OuterSubscriber<any, any>,
result: ObservableInput<T>,
outerValue?: T,
outerIndex?: number): Subscription {
let destination: Subscriber<any> = new InnerSubscriber(outerSubscriber, outerValue, outerIndex);

if (destination.isUnsubscribed) {
return;
}

if (result instanceof Observable) {
if (result._isScalar) {
destination.next(result.value);
destination.next((<any>result).value);
destination.complete();
return;
} else {
Expand All @@ -38,9 +42,9 @@ export function subscribeToResult<T, R>(outerSubscriber: OuterSubscriber<T, R>,
}
} else if (isPromise(result)) {
result.then(
(value: any) => {
(value) => {
if (!destination.isUnsubscribed) {
destination.next(value);
destination.next(<any>value);
destination.complete();
}
},
Expand All @@ -52,8 +56,8 @@ export function subscribeToResult<T, R>(outerSubscriber: OuterSubscriber<T, R>,
});
return destination;
} else if (typeof result[$$iterator] === 'function') {
for (let item of result) {
destination.next(item);
for (let item of <any>result) {
destination.next(<any>item);
if (destination.isUnsubscribed) {
break;
}
Expand Down