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.

Sign up for GitHub

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

Jump to bottom

feat(startWith): remove deprecated scheduler signature. #7166

Merged
merged 5 commits into from May 25, 2023
Merged

feat(startWith): remove deprecated scheduler signature. #7166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d25d94
feat(startWith): Deprecated scheduler signature removed
benlesh Jan 24, 2023
509257d
refactor: Move `subscribeToArray` closer to primary usage.
benlesh Jan 24, 2023
3a1a490
refactor: no longer export `fromArrayLike` from `innerFrom.ts`, to di…
benlesh Jan 24, 2023
6d18c49
chore: Remove old comments.
benlesh Jan 24, 2023
13e19fc
refactor(startWith): use new function
benlesh May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 1 addition & 12 deletions spec-dtslint/operators/startWith-spec.ts
View file
Open in desktop
@@ -1,5 +1,4 @@
import { of, asyncScheduler } from 'rxjs';
import { startWith } from 'rxjs/operators';
import { of, startWith } from 'rxjs';
import { A, B, a, b, c, d, e, f, g, h } from '../helpers';

it('should infer correctly with N values', () => {
Expand All @@ -13,16 +12,6 @@ it('should infer correctly with N values', () => {
const r7 = of(a).pipe(startWith(b, c, d, e, f, g, h)); // $ExpectType Observable<A | B | C | D | E | F | G | H>
});

it('should infer correctly with a scheduler', () => {
const r = of(a).pipe(startWith(asyncScheduler)); // $ExpectType Observable<A>
const r1 = of(a).pipe(startWith(b, asyncScheduler)); // $ExpectType Observable<A | B>
const r2 = of(a).pipe(startWith(b, c, asyncScheduler)); // $ExpectType Observable<A | B | C>
const r3 = of(a).pipe(startWith(b, c, d, asyncScheduler)); // $ExpectType Observable<A | B | C | D>
const r4 = of(a).pipe(startWith(b, c, d, e, asyncScheduler)); // $ExpectType Observable<A | B | C | D | E>
const r5 = of(a).pipe(startWith(b, c, d, e, f, asyncScheduler)); // $ExpectType Observable<A | B | C | D | E | F>
const r6 = of(a).pipe(startWith(b, c, d, e, f, g, asyncScheduler)); // $ExpectType Observable<A | B | C | D | E | F | G>
});

it('should infer correctly with a single specified type', () => {
const r0 = of(a).pipe(startWith<A>(a)); // $ExpectType Observable<A>
const r1 = of(a).pipe(startWith<A|B>(b)); // $ExpectType Observable<A | B>
Expand Down
59 changes: 2 additions & 57 deletions spec/operators/startWith-spec.ts
View file
Open in desktop
@@ -1,7 +1,6 @@
import { expect } from 'chai';
import { startWith, mergeMap, take } from 'rxjs/operators';
import { TestScheduler } from 'rxjs/testing';
import { of, Observable } from 'rxjs';
import { of, Observable, startWith, mergeMap, take } from 'rxjs';
import { observableMatcher } from '../helpers/observableMatcher';

/** @test {startWith} */
Expand Down Expand Up @@ -143,21 +142,6 @@ describe('startWith', () => {
});
});

it('should allow unsubscribing explicitly and early', () => {
testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => {
const e1 = hot(' ---a--b----c--d--|');
const unsub = ' ---------! ';
const e1subs = ' ^--------! ';
const expected = 's--a--b--- ';
const values = { s: 's', a: 'a', b: 'b' };

const result = e1.pipe(startWith('s', testScheduler));
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reinsurance. Are you sure that these tests should be deleted? Isn't it enough just to remove testScheduler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, they're invalid tests now. This would test that it would start with 's' and then an instance of testScheduler. :)


expectObservable(result, unsub).toBe(expected, values);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});
});

it('should not break unsubscription chains when result is unsubscribed explicitly', () => {
testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => {
const e1 = hot(' ---a--b----c--d--|');
Expand All @@ -168,7 +152,7 @@ describe('startWith', () => {

const result = e1.pipe(
mergeMap((x: string) => of(x)),
startWith('s', testScheduler),
startWith('s'),
mergeMap((x: string) => of(x))
);

Expand All @@ -177,45 +161,6 @@ describe('startWith', () => {
});
});

it('should start with empty if given value is not specified', () => {
testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => {
const e1 = hot(' -a-|');
const e1subs = ' ^--!';
const expected = '-a-|';

const result = e1.pipe(startWith(testScheduler));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});
});

it('should accept scheduler as last argument with single value', () => {
testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => {
const e1 = hot(' --a--|');
const e1subs = ' ^----!';
const expected = 'x-a--|';

const result = e1.pipe(startWith(defaultStartValue, testScheduler));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});
});

it('should accept scheduler as last argument with multiple value', () => {
testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => {
const e1 = hot(' -----a--|');
const e1subs = ' ^-------!';
const expected = '(yz)-a--|';

const result = e1.pipe(startWith('y', 'z', testScheduler));

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});
});

it('should stop listening to a synchronous observable when unsubscribed', () => {
const sideEffects: number[] = [];
const synchronousObservable = new Observable<number>((subscriber) => {
Expand Down
7 changes: 5 additions & 2 deletions src/internal/observable/from.ts
View file
Open in desktop
Expand Up @@ -139,7 +139,7 @@ export function fromArrayLike<T>(array: ArrayLike<T>) {
});
}

function fromPromise<T>(promise: PromiseLike<T>) {
export function fromPromise<T>(promise: PromiseLike<T>) {
return new Observable((subscriber: Subscriber<T>) => {
promise
.then(
Expand Down Expand Up @@ -205,7 +205,10 @@ export function subscribeToArray<T>(array: ArrayLike<T>, subscriber: Subscriber<
// be to copy the array before executing the loop, but this has
// performance implications.
const length = array.length;
for (let i = 0; i < length && !subscriber.closed; i++) {
for (let i = 0; i < length; i++) {
if (subscriber.closed) {
return;
}
subscriber.next(array[i]);
}
subscriber.complete();
Expand Down
4 changes: 2 additions & 2 deletions src/internal/operators/endWith.ts
View file
Open in desktop
@@ -1,8 +1,8 @@
/** prettier */
import { subscribeToArray } from '../observable/from';
import { OperatorFunction, ValueFromArray } from '../types';
import { Observable } from '../Observable';
import { OperatorFunction, ValueFromArray } from '../types';
import { operate } from '../Subscriber';
import { subscribeToArray } from '../observable/from';

/**
* Returns an observable that will emit all values from the source, then synchronously emit
Expand Down
24 changes: 5 additions & 19 deletions src/internal/operators/startWith.ts
View file
Open in desktop
@@ -1,20 +1,10 @@
import { concat } from '../observable/concat';
import { OperatorFunction, SchedulerLike, ValueFromArray } from '../types';
import { popScheduler } from '../util/args';
import { Observable } from '../Observable';

// Devs are more likely to pass null or undefined than they are a scheduler
// without accompanying values. To make things easier for (naughty) devs who
// use the `strictNullChecks: false` TypeScript compiler option, these
// overloads with explicit null and undefined values are included.
import { subscribeToArray } from '../observable/from';
import { operate } from '../Subscriber';
import { OperatorFunction, ValueFromArray } from '../types';

export function startWith<T>(value: null): OperatorFunction<T, T | null>;
export function startWith<T>(value: undefined): OperatorFunction<T, T | undefined>;

/** @deprecated The `scheduler` parameter will be removed in v8. Use `scheduled` and `concatAll`. Details: https://rxjs.dev/deprecations/scheduler-argument */
export function startWith<T, A extends readonly unknown[] = T[]>(
...valuesAndScheduler: [...A, SchedulerLike]
): OperatorFunction<T, T | ValueFromArray<A>>;
export function startWith<T, A extends readonly unknown[] = T[]>(...values: A): OperatorFunction<T, T | ValueFromArray<A>>;

/**
Expand Down Expand Up @@ -57,12 +47,8 @@ export function startWith<T, A extends readonly unknown[] = T[]>(...values: A):
* @see {@link concat}
*/
export function startWith<T, D>(...values: D[]): OperatorFunction<T, T | D> {
const scheduler = popScheduler(values);
return (source) =>
new Observable((subscriber) => {
// Here we can't pass `undefined` as a scheduler, because if we did, the
// code inside of `concat` would be confused by the `undefined`, and treat it
// like an invalid observable. So we have to split it two different ways.
(scheduler ? concat(values, source, scheduler) : concat(values, source)).subscribe(subscriber);
new Observable((destination) => {
subscribeToArray(values, operate({ destination, complete: () => source.subscribe(destination) }));
});
}