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

refactor: remove legacy code #7183

Closed
wants to merge 11 commits into from
45 changes: 0 additions & 45 deletions spec/observables/dom/ajax-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,37 +330,6 @@ describe('ajax', () => {
expect(complete).to.be.true;
});

it('should fail if fails to parse response in older IE', () => {
let error: any;
const obj: AjaxConfig = {
url: '/flibbertyJibbet',
method: '',
};

// No `response` property on the object (for older IE).
MockXMLHttpRequest.noResponseProp = true;

ajax(obj).subscribe({
next: () => {
throw new Error('should not next');
},
error: (err: any) => {
error = err;
},
complete: () => {
throw new Error('should not complete');
},
});

MockXMLHttpRequest.mostRecent.respondWith({
status: 207,
responseText: 'Wee! I am text, but should be valid JSON!',
});

expect(error instanceof SyntaxError).to.be.true;
expect(error.message).to.equal('Unexpected token W in JSON at position 0');
});

it('should fail on 404', () => {
let error: any;
const obj: AjaxConfig = {
Expand Down Expand Up @@ -1583,11 +1552,6 @@ class MockXHREventTarget {
class MockXMLHttpRequest extends MockXHREventTarget {
static readonly DONE = 4;

/**
* Set to `true` to test IE code paths.
*/
static noResponseProp = false;

private static requests: Array<MockXMLHttpRequest> = [];
private static recentRequest: MockXMLHttpRequest;

Expand All @@ -1600,7 +1564,6 @@ class MockXMLHttpRequest extends MockXHREventTarget {
}

static clearRequest(): void {
MockXMLHttpRequest.noResponseProp = false;
MockXMLHttpRequest.requests.length = 0;
MockXMLHttpRequest.recentRequest = null!;
}
Expand Down Expand Up @@ -1639,9 +1602,6 @@ class MockXMLHttpRequest extends MockXHREventTarget {
super();
MockXMLHttpRequest.recentRequest = this;
MockXMLHttpRequest.requests.push(this);
if (MockXMLHttpRequest.noResponseProp) {
delete this['response'];
}
}

// @ts-ignore: Property has no initializer and is not definitely assigned
Expand Down Expand Up @@ -1755,11 +1715,6 @@ class MockXMLHttpRequest extends MockXHREventTarget {
break;
}

// We're testing old IE, forget all of that response property stuff.
if (MockXMLHttpRequest.noResponseProp) {
delete this['response'];
}

this.triggerEvent('load', { type: 'load', total: response.total ?? 0, loaded: response.loaded ?? 0 });
this.triggerEvent('readystatechange', { type: 'readystatechange' });
}
Expand Down
31 changes: 11 additions & 20 deletions spec/operators/groupBy-spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { expect } from 'chai';
import { groupBy, delay, tap, map, take, mergeMap, materialize, skip, ignoreElements } from 'rxjs/operators';
import { TestScheduler } from 'rxjs/testing';
import { ReplaySubject, of, Observable, Operator, Observer, Subject, NextNotification, ErrorNotification } from 'rxjs';
import { createNotification } from 'rxjs/internal/NotificationFactories';
import { ReplaySubject, of, Observable, Operator, Observer, Subject, OperatorFunction, pipe, ObservableNotification } from 'rxjs';
import { observableMatcher } from '../helpers/observableMatcher';

/** @test {groupBy} */
Expand Down Expand Up @@ -1537,22 +1536,14 @@ describe('groupBy operator', () => {
/**
* TODO: A helper operator to deal with legacy tests above that could probably be written a different way
*/
function phonyMarbelize<T>(testScheduler: TestScheduler) {
return (source: Observable<T>) =>
source.pipe(
materialize(),
map((notification) => {
// Because we're hacking some weird inner-observable marbles here, we need
// to make sure this is all the same shape as it would be from the TestScheduler
// assertions
return {
frame: testScheduler.frame,
notification: createNotification(
notification.kind,
(notification as NextNotification<T>).value,
(notification as ErrorNotification).error
),
};
})
);
function phonyMarbelize<T>(testScheduler: TestScheduler): OperatorFunction<T, { frame: number; notification: ObservableNotification<T> }> {
return pipe(
materialize(),
map((notification) => {
// Because we're hacking some weird inner-observable marbles here, we need
// to make sure this is all the same shape as it would be from the TestScheduler
// assertions
return { frame: testScheduler.frame, notification };
})
);
}
5 changes: 5 additions & 0 deletions spec/util/UnsubscriptionError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ describe('UnsubscriptionError', () => {
expect(err.errors).to.deep.equal([err1, err2]);
expect(err.name).to.equal('UnsubscriptionError');
expect(err.stack).to.be.a('string');
expect(err.message).to.equal(
`2 errors occurred during unsubscription:
1) Error: Swiss cheese tastes amazing but smells like socks
2) Error: User too big to fit in tiny European elevator`
);
} else {
throw new TypeError('Invalid error type');
}
Expand Down
27 changes: 0 additions & 27 deletions spec/util/createErrorClass-spec.ts

This file was deleted.

33 changes: 5 additions & 28 deletions src/internal/NotificationFactories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,22 @@ import { CompleteNotification, NextNotification, ErrorNotification } from './typ
* same "shape" as other notifications in v8.
* @internal
*/
export const COMPLETE_NOTIFICATION = (() => createNotification('C', undefined, undefined) as CompleteNotification)();
export const COMPLETE_NOTIFICATION: CompleteNotification = { kind: 'C' };

/**
* Internal use only. Creates an optimized error notification that is the same "shape"
* as other notifications.
* @internal
*/
export function errorNotification(error: any): ErrorNotification {
return createNotification('E', undefined, error) as any;
export function errorNotification(error: unknown): ErrorNotification {
return { kind: 'E', error };
}

/**
* Internal use only. Creates an optimized next notification that is the same "shape"
* as other notifications.
* @internal
*/
export function nextNotification<T>(value: T) {
return createNotification('N', value, undefined) as NextNotification<T>;
}

export function createNotification<T>(kind: 'N', value: T, error: undefined): { kind: 'N'; value: T; error: undefined };
export function createNotification<T>(kind: 'E', value: undefined, error: any): { kind: 'E'; value: undefined; error: any };
export function createNotification<T>(kind: 'C', value: undefined, error: undefined): { kind: 'C'; value: undefined; error: undefined };
export function createNotification<T>(
kind: 'N' | 'E' | 'C',
value: T | undefined,
error: any
): { kind: 'N' | 'E' | 'C'; value: T | undefined; error: any };

/**
* Ensures that all notifications created internally have the same "shape" in v8.
*
* TODO: This is only exported to support a crazy legacy test in `groupBy`.
* @internal
*/
export function createNotification<T>(kind: 'N' | 'E' | 'C', value: T | undefined, error: any) {
return {
kind,
value,
error,
};
export function nextNotification<T>(value: T): NextNotification<T> {
return { kind: 'N', value };
}
2 changes: 1 addition & 1 deletion src/internal/Subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class Subscription implements SubscriptionLike {
* @return {void}
*/
unsubscribe(): void {
let errors: any[] | undefined;
let errors: unknown[] | undefined;

if (!this.closed) {
this.closed = true;
Expand Down
13 changes: 1 addition & 12 deletions src/internal/ajax/ajax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,7 @@ export function fromAjax<T>(init: AjaxConfig): Observable<AjaxResponse<T>> {
if (status < 400) {
progressSubscriber?.complete?.();

let response: AjaxResponse<T>;
try {
// This can throw in IE, because we end up needing to do a JSON.parse
// of the response in some cases to produce object we'd expect from
// modern browsers.
response = createResponse(DOWNLOAD, event);
} catch (err) {
destination.error(err);
return;
}

destination.next(response);
destination.next(createResponse(DOWNLOAD, event));
destination.complete();
} else {
progressSubscriber?.error?.(event);
Expand Down
82 changes: 18 additions & 64 deletions src/internal/ajax/errors.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { AjaxRequest } from './types';
import { getXHRResponse } from './getXHRResponse';
import { createErrorClass } from '../util/createErrorClass';

/**
* A normalized AJAX error.
* Thrown when an error occurs during an AJAX request.
*
* @see {@link ajax}
*
* @class AjaxError
*/
export interface AjaxError extends Error {
export class AjaxError extends Error {
name = 'AjaxError';

/**
* The XHR instance associated with the error.
*/
Expand All @@ -35,72 +34,27 @@ export interface AjaxError extends Error {
* The response data.
*/
response: any;
}

export interface AjaxErrorCtor {
/**
* @deprecated Internal implementation detail. Do not construct error instances.
* Cannot be tagged as internal: https://github.com/ReactiveX/rxjs/issues/6269
*/
new (message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError;
}

/**
* Thrown when an error occurs during an AJAX request.
* This is only exported because it is useful for checking to see if an error
* is an `instanceof AjaxError`. DO NOT create new instances of `AjaxError` with
* the constructor.
*
* @class AjaxError
* @see {@link ajax}
*/
export const AjaxError: AjaxErrorCtor = createErrorClass(
(_super) =>
function AjaxErrorImpl(this: any, message: string, xhr: XMLHttpRequest, request: AjaxRequest) {
this.message = message;
this.name = 'AjaxError';
this.xhr = xhr;
this.request = request;
this.status = xhr.status;
this.responseType = xhr.responseType;
let response: any;
try {
// This can throw in IE, because we have to do a JSON.parse of
// the response in some cases to get the expected response property.
response = getXHRResponse(xhr);
} catch (err) {
response = xhr.responseText;
}
this.response = response;
}
);

export interface AjaxTimeoutError extends AjaxError {}
constructor(message: string, xhr: XMLHttpRequest, request: AjaxRequest) {
super(message);

export interface AjaxTimeoutErrorCtor {
/**
* @deprecated Internal implementation detail. Do not construct error instances.
* Cannot be tagged as internal: https://github.com/ReactiveX/rxjs/issues/6269
*/
new (xhr: XMLHttpRequest, request: AjaxRequest): AjaxTimeoutError;
this.xhr = xhr;
this.request = request;
this.status = xhr.status;
this.responseType = xhr.responseType;
this.response = getXHRResponse(xhr);
}
}

/**
* Thrown when an AJAX request times out. Not to be confused with {@link TimeoutError}.
*
* This is exported only because it is useful for checking to see if errors are an
* `instanceof AjaxTimeoutError`. DO NOT use the constructor to create an instance of
* this type.
*
* @class AjaxTimeoutError
* @see {@link ajax}
*/
export const AjaxTimeoutError: AjaxTimeoutErrorCtor = (() => {
function AjaxTimeoutErrorImpl(this: any, xhr: XMLHttpRequest, request: AjaxRequest) {
AjaxError.call(this, 'ajax timeout', xhr, request);
this.name = 'AjaxTimeoutError';
return this;
export class AjaxTimeoutError extends AjaxError {
name = 'AjaxTimeoutError';

constructor(xhr: XMLHttpRequest, request: AjaxRequest) {
super('ajax timeout', xhr, request);
}
AjaxTimeoutErrorImpl.prototype = Object.create(AjaxError.prototype);
return AjaxTimeoutErrorImpl;
})() as any;
}
30 changes: 2 additions & 28 deletions src/internal/ajax/getXHRResponse.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,11 @@
/**
* Gets what should be in the `response` property of the XHR. However,
* since we still support the final versions of IE, we need to do a little
* checking here to make sure that we get the right thing back. Consequently,
* we need to do a JSON.parse() in here, which *could* throw if the response
* isn't valid JSON.
* Gets what should be in the `response` property of the XHR.
*
* This is used both in creating an AjaxResponse, and in creating certain errors
* that we throw, so we can give the user whatever was in the response property.
*
* @param xhr The XHR to examine the response of
*/
export function getXHRResponse(xhr: XMLHttpRequest) {
switch (xhr.responseType) {
case 'json': {
if ('response' in xhr) {
return xhr.response;
} else {
// IE
const ieXHR: any = xhr;
return JSON.parse(ieXHR.responseText);
}
}
case 'document':
return xhr.responseXML;
case 'text':
default: {
if ('response' in xhr) {
return xhr.response;
} else {
// IE
const ieXHR: any = xhr;
return ieXHR.responseText;
}
}
}
return xhr.responseType === 'document' ? xhr.responseXML : xhr.response;
}
Loading