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

fix(AjaxObservable): drop resultSelector support in ajax method #1813

Merged
merged 1 commit into from
Jul 17, 2016
Merged

fix(AjaxObservable): drop resultSelector support in ajax method #1813

merged 1 commit into from
Jul 17, 2016

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Jul 8, 2016

Description:

This PR drops support of resultSelector in ajax*() mehod.

  • by removing resultSelector, ajax method delivers strict AjaxResponse type resolve type inference issues on subscription.
  • when it's needed to select specific values, encourage chain map as necessary.

Related issue (if exists):

closes #1783

BREAKING CHANGE: ajax.*() method no longer support resultSelector, encourage to use map instead

@kwonoj
Copy link
Member Author

kwonoj commented Jul 8, 2016

/cc @david-driscoll for visibility.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling 5a9ca15 on kwonoj:ajax-resultselector into 8befc1e on ReactiveX:master.

@kwonoj kwonoj mentioned this pull request Jul 8, 2016
Closed
@jayphelps
Copy link
Member

jayphelps commented Jul 9, 2016 •
edited
Loading

LGTM

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling ed60a4c on kwonoj:ajax-resultselector into 0fc88b2 on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling cbce3b6 on kwonoj:ajax-resultselector into 3632489 on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling 786d05e on kwonoj:ajax-resultselector into 4971993 on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling f962287 on kwonoj:ajax-resultselector into 19f4362 on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.481% when pulling 6988a3d on kwonoj:ajax-resultselector into ffcb9fc on ReactiveX:master.

@jayphelps
Copy link
Member

LGTM

@jayphelps
Copy link
Member

jayphelps commented Jul 17, 2016 •
edited
Loading

Needs rebase, then I'll merge. Ping me.

@kwonoj
fix(AjaxObservable): drop resultSelector support in ajax method
7a77437 
closes ReactiveX#1783

BREAKING CHANGE: ajax.*() method no longer support resultSelector, encourage to use `map` instead
@coveralls
Copy link

coveralls commented Jul 17, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.01%) to 96.482% when pulling 786d05e on kwonoj:ajax-resultselector into 4971993 on ReactiveX:master.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 17, 2016

Thanks, rebased.

@jayphelps jayphelps merged commit 75a355f into ReactiveX:master Jul 17, 2016
@kwonoj kwonoj deleted the ajax-resultselector branch July 17, 2016 03:21
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ajax() returns Observable<{}> instead of Observable<AjaxResponse>
4 participants
@kwonoj @coveralls @jayphelps @benlesh