-
Notifications
You must be signed in to change notification settings - Fork 26.6k
fix(http): introduce named type for HttpParams options #19360
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
Conversation
You can preview 9e661cd at https://pr19360-9e661cd.ngbuilds.io/. |
I suppose it would be too difficult to add test coverage in this PR... |
9e661cd
to
72276f9
Compare
Updated formatting and api gold |
You can preview 72276f9 at https://pr19360-72276f9.ngbuilds.io/. |
packages/common/http/src/params.ts
Outdated
@@ -73,6 +73,12 @@ interface Update { | |||
op: 'a'|'d'|'s'; | |||
} | |||
|
|||
export interface HttpParamsOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that this is a public api, we need an api doc snippet for it.
@jelbourn can you please add a short one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added docs.
This is necessary to enable type-based optimizations with Closure. Without explicity making these options the same named type, Closure thinks they are different types and cannot disambiguate the `fromObject` property.
72276f9
to
c78acf0
Compare
You can preview c78acf0 at https://pr19360-c78acf0.ngbuilds.io/. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This is necessary to enable type-based optimizations with Closure.
Without explicity making these options the same named type, Closure
thinks they are different types and cannot disambiguate the
fromObject
property.
@alxhub @alexeagle let's discuss this together Monday morning