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.

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

Jump to bottom

The Request constructor doesn't respect the deprecated RequestOptionsArgs.search property #15761

Closed
hristokostov opened this issue Apr 4, 2017 · 5 comments
Closed

The Request constructor doesn't respect the deprecated RequestOptionsArgs.search property #15761

hristokostov opened this issue Apr 4, 2017 · 5 comments

Comments

@hristokostov
Copy link

I'm submitting a ... (check one with "x")

[ x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior
When creating a new instance of the Request class, the value of the search property of the RequestOptionsArgs object that is passed to the constructor is not respected, i.e. the Request instance has no query parameters. If we use the new params property, query parameters are properly set.

Expected behavior
It is expected the now deprecated property to works too.

Minimal reproduction of the problem with instructions
Here is the plunk: https://plnkr.co/edit/udWvbiYOwLzx29F974jK

  • Angular version: 4.0.X
@DzmitryShylovich
Copy link
Contributor

#15666 (comment)

@hristokostov
Copy link
Author

Thanks for the quick reply.

OK, but how could anyone know that the RequestOptions class exists and have to be used? The Request constructor signature expects RequestOptionsArgs and the documentation example is like this:

 return this.http.request(new Request({
      method: RequestMethod.Get,
      url: url,
      search: 'password=123'
    }));

Today I updated to v4 and my app just broke.

Request wasn't designed to work with RequestOptionsArgs directly.

You mean "was never designed to work with RequestOptionsArgs" or that it was redesigned in the v4 phase? Because I believe the signature/documentation has always been like it is now.

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Apr 4, 2017 •
edited

RequestOptions doesn't implement RequestOptionsArgs. There's no a single test that uses RequestOptionsArgs. There're a lot of inconsistencies. In 4.1 http module will be rewritten and I hope all these issues will fixed.

Right now you have 2 options:

  1. use RequestOptionsArgs with search params
  2. use RequestOptions with params or search

@hristokostov
Copy link
Author

Currently option 1 (passing RequestOptionsArgs with search) is not working. In order to fix my app I just switched from search to params.

RequestOptions with search is fine (as you have pointed in the 15666 comment).

@alxhub alxhub added this to Legacy Bugs in Http Apr 5, 2017
alxhub added a commit to alxhub/angular that referenced this issue Apr 27, 2017
@alxhub
fix(http): honor RequestArgs.search and RequestArgs.params map type
d429787 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761
@alxhub alxhub mentioned this issue Apr 27, 2017
Closed
alxhub added a commit to alxhub/angular that referenced this issue Apr 28, 2017
@alxhub
fix(http): honor RequestArgs.search and RequestArgs.params map type
d3a3a57 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761
@alxhub alxhub moved this from Legacy Bugs to PR Pending in Http Apr 28, 2017
alxhub added a commit to alxhub/angular that referenced this issue May 2, 2017
@alxhub
fix(http): honor RequestArgs.search and RequestArgs.params map type
b05ab38 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761
alxhub added a commit to alxhub/angular that referenced this issue May 3, 2017
@alxhub
fix(http): honor RequestArgs.search and RequestArgs.params map type
f8c9b45 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761
matsko pushed a commit that referenced this issue May 4, 2017
@alxhub @matsko
fix(http): honor RequestArgs.search and RequestArgs.params map type
3138cb1 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes #15761

PR Close #16392
@matsko matsko closed this as completed in aef5245 May 5, 2017
jasonaden pushed a commit to jasonaden/angular that referenced this issue May 10, 2017
@alxhub @jasonaden
fix(http): honor RequestArgs.search and RequestArgs.params map type
63066f7 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761

PR Close angular#16392
@alxhub alxhub removed this from PR Pending in Http May 15, 2017
asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
@alxhub
fix(http): honor RequestArgs.search and RequestArgs.params map type
8cab5d4 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761

PR Close angular#16392
juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
@alxhub @juleskremer
fix(http): honor RequestArgs.search and RequestArgs.params map type
86f882a 
Currently `new Request({search: ...})` is not honored, and
`new Request({params: {'x': 'y'}) doesn't work either, as this object would have
toString() called. This change allows both of these cases to work, as proved by
the 2 new tests.

Fixes angular#15761

PR Close angular#16392
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(http): honor RequestArgs.search and RequestArgs.params map type alxhub/angular
3 participants
@hristokostov @alxhub @DzmitryShylovich