feat(http): serialize search parameters from request options#3020
feat(http): serialize search parameters from request options#3020caitp wants to merge 1 commit intoangular:masterfrom
Conversation
|
@caitp is there anything blocking this PR? |
|
The test hasn't been passing yet, so that's one |
|
@caitp I meant are you working on this, and is anything blocking you from finishing :) |
@jeffbcross I've just pushed a new version which passes each unit test task locally, so this one is probably ready for review. edit "tests" are passing everywhere, but Dartanalyzer fails because the unused imports stuff. Is there any way to prevent those warnings from being emitted while still getting all the typeinfo benefits in TypeScript? |
|
needs info from TypeScript/Dart-knowledgable people |
There was a problem hiding this comment.
I'm surprised enforce-format and lint didn't complain about this
jeffbcross
added
feature
|
Left some comments on requested behavior changes. LMK if you disagree. |
caitp
force-pushed
the
issue-2417
branch
4 times, most recently
from
8ceb463 to
7ff4120
Compare
caitp
added
action: review
There was a problem hiding this comment.
Looks like this isn't used anywhere, and is also not implemented in JS facade. Please remove.
There was a problem hiding this comment.
there might be a few other bits of cleanup to do, do you see anything that looks more like a "this won't work or doesn't do what we want" kind of thing?
There was a problem hiding this comment.
or if you can spot missing test coverage I'll add some more tests before checking in
jeffbcross
added
action: cleanup
|
Looks good with some cleanup and squashing. |
jeffbcross
added
pr_state: LGTM
action: cleanup
- Extends URLSearchParams API to include operations for combining
different URLSearchParams objects:
These new methods include:
setAll(otherParams): performs `this.set(key, values[0])` for each
key/value-list pair in `otherParams`
appendAll(otherParams): performs `this.append(key, values)` for
each key/value-list pair in `otherParams`
replaceAll(otherParams): for each key/value-list pair in
`otherParams`, replaces current set of values for `key` with
a copy of the list of values.
- RequestOptions do not merge search params automatically (because
there are multiple ways to do this). Instead, they replace any
existing `search` field if `search` is provided. Explicit merging
is required if merging is desirable.
- Some extra test coverage added.
Closes angular#2417
caitp
added
action: merge
|
User @caitp does not have PR merging privlidges. |
mary-poppins
removed
the
action: merge
|
@caitp the merge label is special now. For whitelisted committers, it will put the PR in the presubmit queue as-is. When you're ready to merge it, you can just push to upstream presubmit-caitp-pr-3020 |
tbosch
added
action: merge
- Extends URLSearchParams API to include operations for combining
different URLSearchParams objects:
These new methods include:
setAll(otherParams): performs `this.set(key, values[0])` for each
key/value-list pair in `otherParams`
appendAll(otherParams): performs `this.append(key, values)` for
each key/value-list pair in `otherParams`
replaceAll(otherParams): for each key/value-list pair in
`otherParams`, replaces current set of values for `key` with
a copy of the list of values.
- RequestOptions do not merge search params automatically (because
there are multiple ways to do this). Instead, they replace any
existing `search` field if `search` is provided. Explicit merging
is required if merging is desirable.
- Some extra test coverage added.
Closes angular#2417
Closes angular#3020
|
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. |
6 participants
WIP
Closes #2417