Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

feat(common): support custom JSON parser in HttpClient #25027

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

Closed
trotyl wants to merge 1 commit into from
Closed

feat(common): support custom JSON parser in HttpClient #25027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion 8 goldens/public-api/common/http/http.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1725,14 +1725,20 @@ export declare interface HttpUserEvent<T> {
}

export declare class HttpXhrBackend implements HttpBackend {
constructor(xhrFactory: XhrFactory);
constructor(xhrFactory: XhrFactory, jsonParser: JsonParser);
handle(req: HttpRequest<any>): Observable<HttpEvent<any>>;
}

export declare abstract class HttpXsrfTokenExtractor {
abstract getToken(): string | null;
}

export declare const JSON_PARSER: InjectionToken<JsonParser>;

export interface JsonParser {
parse(text: string, reviver?: (key: any, value: any) => any): any;
}

export declare class JsonpClientBackend implements HttpBackend {
constructor(callbackMap: ɵangular_packages_common_http_http_b, document: any);
handle(req: HttpRequest<never>): Observable<HttpEvent<any>>;
Expand Down
2 changes: 1 addition & 1 deletion 2 packages/common/http/public_api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ export {HttpClientJsonpModule, HttpClientModule, HttpClientXsrfModule, HttpInter
export {HttpParameterCodec, HttpParams, HttpParamsOptions, HttpUrlEncodingCodec} from './src/params';
export {HttpRequest} from './src/request';
export {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType, HttpHeaderResponse, HttpProgressEvent, HttpResponse, HttpResponseBase, HttpSentEvent, HttpUploadProgressEvent, HttpUserEvent} from './src/response';
export {HttpXhrBackend, XhrFactory} from './src/xhr';
export {HttpXhrBackend, JSON_PARSER, JsonParser, XhrFactory} from './src/xhr';
export {HttpXsrfTokenExtractor} from './src/xsrf';
7 changes: 6 additions & 1 deletion 7 packages/common/http/src/module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {HTTP_INTERCEPTORS, HttpInterceptor, HttpInterceptorHandler, NoopIntercep
import {JsonpCallbackContext, JsonpClientBackend, JsonpInterceptor} from './jsonp';
import {HttpRequest} from './request';
import {HttpEvent} from './response';
import {BrowserXhr, HttpXhrBackend, XhrFactory} from './xhr';
import {BrowserXhr, HttpXhrBackend, JSON_PARSER, JsonParser, XhrFactory} from './xhr';
import {HttpXsrfCookieExtractor, HttpXsrfInterceptor, HttpXsrfTokenExtractor, XSRF_COOKIE_NAME, XSRF_HEADER_NAME} from './xsrf';

/**
Expand Down Expand Up @@ -131,6 +131,10 @@ export class HttpClientXsrfModule {
}
}

export function _JSON(): JsonParser {
return JSON;
}
Comment on lines +134 to +136
Copy link
Contributor

@petebacondarwin petebacondarwin Jan 26, 2021

Choose a reason for hiding this comment

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

If jsonParser was optional then this would not be needed.


/**
* Configures the [dependency injector](guide/glossary#injector) for `HttpClient`
* with supporting services for XSRF. Automatically imported by `HttpClientModule`.
Expand Down Expand Up @@ -161,6 +165,7 @@ export class HttpClientXsrfModule {
{provide: HttpBackend, useExisting: HttpXhrBackend},
BrowserXhr,
{provide: XhrFactory, useExisting: BrowserXhr},
{provide: JSON_PARSER, useFactory: _JSON, deps: []},
],
})
export class HttpClientModule {
Expand Down
14 changes: 11 additions & 3 deletions 14 packages/common/http/src/xhr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Injectable} from '@angular/core';
import {Inject, Injectable, InjectionToken} from '@angular/core';
import {Observable, Observer} from 'rxjs';

import {HttpBackend} from './backend';
Expand All @@ -16,6 +16,13 @@ import {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType,

const XSSI_PREFIX = /^\)\]\}',?\n/;

/**
* Utility for parsing JSON text to JavaScript object
Copy link
Contributor

@petebacondarwin petebacondarwin Jan 26, 2021

Choose a reason for hiding this comment

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

I think we would need a bit more documentation for this and the JSON_PARSER token below, ideally with an example and referencing each other.

*/
export interface JsonParser { parse(text: string, reviver?: (key: any, value: any) => any): any; }

export const JSON_PARSER = new InjectionToken<JsonParser>('JsonParser');

/**
* Determine an appropriate URL for the response, by checking either
* XMLHttpRequest.responseURL or the X-Request-URL header.
Expand Down Expand Up @@ -70,7 +77,8 @@ interface PartialResponse {
*/
@Injectable()
export class HttpXhrBackend implements HttpBackend {
constructor(private xhrFactory: XhrFactory) {}
constructor(private xhrFactory: XhrFactory, @Inject(JSON_PARSER) private jsonParser: JsonParser) {
Copy link
Contributor

@petebacondarwin petebacondarwin Jan 26, 2021

Choose a reason for hiding this comment

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

How about making this optional, then we wouldn't have to define a provider for the default case:

Suggested change
constructor(private xhrFactory: XhrFactory, @Inject(JSON_PARSER) private jsonParser: JsonParser) {
constructor(private xhrFactory: XhrFactory, @Optional() @Inject(JSON_PARSER) private jsonParser?: JsonParser) {
if (jsonParser == null) { // note == matches both `null` and `undefined`
this.jsonParser = JSON;
}

}

/**
* Processes a request and returns a stream of response events.
Expand Down Expand Up @@ -192,7 +200,7 @@ export class HttpXhrBackend implements HttpBackend {
body = body.replace(XSSI_PREFIX, '');
try {
// Attempt the parse. If it fails, a parse error should be delivered to the user.
body = body !== '' ? JSON.parse(body) : null;
body = body !== '' ? this.jsonParser.parse(body) : null;
} catch (error) {
// Since the JSON.parse failed, it's reasonable to assume this might not have been a
// JSON response. Restore the original body (including any XSSI prefix) to deliver
Expand Down
17 changes: 15 additions & 2 deletions 17 packages/common/http/test/xhr_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {HttpRequest} from '@angular/common/http/src/request';
import {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType, HttpHeaderResponse, HttpResponse, HttpResponseBase, HttpUploadProgressEvent} from '@angular/common/http/src/response';
import {HttpXhrBackend} from '@angular/common/http/src/xhr';
import {HttpXhrBackend, JsonParser} from '@angular/common/http/src/xhr';
import {ddescribe, describe, fit, it} from '@angular/core/testing/src/testing_internal';
import {Observable} from 'rxjs';
import {toArray} from 'rxjs/operators';
Expand All @@ -33,7 +33,7 @@ const XSSI_PREFIX = ')]}\'\n';
let backend: HttpXhrBackend = null!;
beforeEach(() => {
factory = new MockXhrFactory();
backend = new HttpXhrBackend(factory);
backend = new HttpXhrBackend(factory, JSON);
Copy link
Contributor

@petebacondarwin petebacondarwin Jan 26, 2021

Choose a reason for hiding this comment

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

If jsonParser was optional this would not need to change.

});
it('emits status immediately', () => {
const events = trackEvents(backend.handle(TEST_POST));
Expand Down Expand Up @@ -94,6 +94,19 @@ const XSSI_PREFIX = ')]}\'\n';
const res = events[1] as HttpResponse<{data: string}>;
expect(res.body!.data).toBe('some data');
});
it('supports custom json parser', () => {
const parser: JsonParser = {
parse() {
return 'JSON_RESULT';
}
};
backend = new HttpXhrBackend(factory, parser);
const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'})));
factory.mock.mockFlush(200, 'OK', JSON.stringify({data: 'NOT USED'}));
expect(events.length).toBe(2);
const res = events[1] as HttpResponse<string>;
expect(res.body).toBe('JSON_RESULT');
});
it('handles a blank json response', () => {
const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'})));
factory.mock.mockFlush(200, 'OK', '');
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.