feat(common): support custom JSON parser in HttpClient

[trotyl]

closes #21079

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #21079

When user works with an extended JSON format, eg. JSON5, due to it's not compatible with JSON.parse(), user have to make it raw text and parse it manually.

But JSON5 format is still conceptually 'json' rather than 'text', it's a fully superset of ES JSON, just being more human friendly.

What is the new behavior?

Simply with:

@NgModule({
  providers: [
    { provider: JsonParser, useValue: JSON5 }
  ]
})
class AppModule {}

Or strip a different receiver:

class MyJsonParser implements JsonParser {
  parse(text: string, reviver?: (key: any, value: any) => any): any {
    const strippedText = text.trimStart(CUSTOM_XSSI)
    return JSON.parse(strippedText, reviver)
  }
}

Or have a different reviver:

class MyJsonParser implements JsonParser {
  parse(text: string, reviver?: (key: any, value: any) => any): any {
    const wrappedReviver = wrapReviver(reviver)
    return JSON.parse(text, wrappedReviver)
  }
}

Then server could provide JSON5 response directly and no error would be raised.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[Lonli-Lokli]

What should be done to accept this PR?

[petebacondarwin]

@trotyl thanks for this PR. I am sorry it slipped under our radar for so long. I rebased it and cleaned up the conflicts. But I have a few suggestions for how to make it a bit slimmer.

[petebacondarwin]

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;
	}

[petebacondarwin]

If jsonParser was optional this would not need to change.

[petebacondarwin]

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.

[petebacondarwin]

If jsonParser was optional then this would not be needed.

[petebacondarwin]

We discussed this in the team meeting yesterday. The general consensus is that this is actually fairly straightforward to do via an interceptor and that the real problem is the lack of discoverability of this approach to solving the problem.

We accept that the problem is compounded by the fact that HttpClient defaults to json parsing with its own parser and so the natural solution that people lean towards is replacing this built-in JSON parsing directly. It would have been better if the default had been text so that JSON parsing felt more like a special case that would lead more naturally to overriding via an interceptor.

The proposal then, is not to merge this feature into core, but instead to improve the documentation to lead developers clearly to an interceptor based solution.

Here is a link to an example of how custom JSON parseing could be done in a reusable interceptor: https://stackblitz.com/edit/angular-ivy-nrdj5q?file=src%2Fapp%2Fapp.module.ts. Note that this could be wrapped up and distributed as a 3rd party library and then the application developer would only need to provide their own JsonParser class.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}