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.

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

v7: ajax: include progress events in return type #6145

Closed
OliverJAsh opened this issue Mar 16, 2021 · 4 comments
Closed

v7: ajax: include progress events in return type #6145

OliverJAsh opened this issue Mar 16, 2021 · 4 comments

Comments

@OliverJAsh
Copy link
Contributor

Bug Report

Current Behavior
Extracted from PR comment: #6001 (comment)

import { ajax } from 'rxjs/ajax';

// Expected return type: Observable<AjaxUploadProgressEvents | AjaxDownloadCompleteEvent | AjaxResponse<T>>
// Actual return type: Observable<AjaxResponse<unknown>>
const ob$ = ajax({
    method: 'POST',
    url: 'https://httpbin.org/post',
    includeUploadProgress: true,
});

ob$.subscribe(value => {
    // Expected type: AjaxUploadProgressEvents | AjaxDownloadCompleteEvent | AjaxResponse<T>
    // Actual type: AjaxResponse<unknown>
    value;
})

Expected behavior
See comments in code above.

Reproduction
See above.

@cartant cartant added the TS Issues and PRs related purely to TypeScript issues label Mar 17, 2021
@cartant
Copy link
Collaborator

cartant commented Mar 17, 2021

@benlesh Beyond the includeUploadProgress-related types, there are types mentioned in the docs that don't exist (AjaxProgressEvents and AjaxDownloadCompleteEvent):

/**
* If `true`, will emit all download progress and load complete events as {@link AjaxProgressEvents}
* from the observable. The final download event will also be emitted as a {@link AjaxDownloadCompleteEvent}
*
* If both this and {@link includeUploadProgress} are `false`, then only the {@link AjaxResponse} will
* be emitted from the resulting observable.
*/
includeDownloadProgress?: boolean;

and the docs for includeUploadProgress mention download events, etc.:

/**
* If `true`, will emit all upload progress and load complete events as {@link AjaxUploadProgressEvents}
* from the observable. The final download event will also be emitted as a {@link AjaxDownloadCompleteEvent}
*
* If both this and {@link includeDownloadProgress} are `false`, then only the {@link AjaxResponse} will
* be emitted from the resulting observable.
*/
includeUploadProgress?: boolean;

@benlesh
Copy link
Member

benlesh commented Apr 20, 2021

This merged PR is related: #6233

@cartant
Copy link
Collaborator

cartant commented Apr 27, 2021

AFAICT, the consumer always receives an AjaxResponse and the type of the response can be inspected to see whether it's a progress event or whatever.

/**
* Creates a response object to emit to the consumer.
* @param direction the direction related to the event. Prefixes the event `type` in the
* `AjaxResponse` object with "upload_" for events related to uploading and "download_"
* for events related to downloading.
* @param event the actual event object.
*/
const createResponse = (direction: AjaxDirection, event: ProgressEvent) =>
new AjaxResponse<T>(event, xhr, _request, `${direction}_${event.type}`);

/**
* The event type. This can be used to discern between different events
* if you're using progress events with {@link includeDownloadProgress} or
* {@link includeUploadProgress} settings in {@link AjaxConfig}.
*
* The event type consists of two parts: the {@link AjaxDirection} and the
* the event type. Merged with `_`, they form the `type` string. The
* direction can be an `upload` or a `download` direction, while an event can
* be `loadstart`, `progress` or `load`.
*
* `download_load` is the type of event when download has finished and the
* response is available.
*/
public readonly type: string = 'download_load'

However, I've ascertained this by reading the code. The ajax observable is something that I've not used in a long while. I'm guessing this can be closed now?

@cartant cartant removed the TS Issues and PRs related purely to TypeScript issues label Apr 27, 2021
@OliverJAsh
Copy link
Contributor Author

Ah, this comment is what lead me to believe the observable could emit types other than AjaxResponse.

https://github.com/ReactiveX/rxjs/pull/6001/files#diff-3345eb17e0a6c80853bd0dfb6dd88d6775cc0dac8c5a4bd4d7113942db4ddfa6R191-R195

It looks like that comment has since been updated: 40206e5#diff-3345eb17e0a6c80853bd0dfb6dd88d6775cc0dac8c5a4bd4d7113942db4ddfa6L182

I'll close this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants