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

fix: allow HttpFormData in NSHTTPXMLHttpRequest #74

Merged
merged 1 commit into from
Jul 9, 2022

Conversation

edusperoni
Copy link
Contributor

PR Checklist

What is the current behavior?

If you use a custom HttpRequest for send an HttpFormData, the body is dropped

What is the new behavior?

HttpFormData is correctly sent as the request content

@jerbob92
Copy link
Contributor

jerbob92 commented Jul 9, 2022

Hi,

Thanks for your contribution! I'm not quite sure about merging this yet. The readme already states that this does not work:

Note: this does not work with the Angular HTTPClient, because it tries to transform the HTTPFormData to json. Use the request() method for Multipart posting.

Your changes probably work, but only when you use the NativeScriptHttpClientModule, not when you use the normal integration.

@edusperoni
Copy link
Contributor Author

edusperoni commented Jul 9, 2022

@jerbob92 This actually works if you build your custom request instead of using angular's helper methods (.get, .post etc), but the request itself will fail because this condition is missing.

Example:

Create a custom HttpRequest

export class RawHttpRequest<T> extends HttpRequest<T> {
  serializeBody(): string | ArrayBuffer | Blob | FormData {
    if (this.body instanceof HTTPFormData) {
      return this.body as any;
    } else {
      return super.serializeBody();
    }
  }

  detectContentTypeHeader(): string | null {
    if (this.body instanceof HTTPFormData) {
      return undefined;
    } else {
      return super.detectContentTypeHeader();
    }
  }

  clone<V = T>(update?: {
    headers?: HttpHeaders;
    context?: HttpContext;
    reportProgress?: boolean;
    params?: HttpParams;
    responseType?: 'arraybuffer' | 'blob' | 'json' | 'text';
    withCredentials?: boolean;
    body?: V;
    method?: string;
    url?: string;
    setHeaders?: { [name: string]: string | string[] };
    setParams?: { [param: string]: string };
  }): HttpRequest<V> {
    const res = super.clone(update);
    return new RawHttpRequest(res.method, res.url, res.body, {
      params: res.params,
      headers: res.headers,
      context: res.context,
      reportProgress: res.reportProgress,
      responseType: res.responseType,
      withCredentials: res.withCredentials,
    });
  }
}

and then you can do:

this.http.request(new RawHttpRequest<MyResponse>('GET', url, klippaFormData, ...);

You can use the normal angular methods if you want as long as you write an interceptor that remaps your request to a RawHttpRequest.

export class FormDataInterceptorService implements HttpInterceptor {
  public intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    if (req.body instanceof HTTPFormData) {
      return next.handle(
        new RawHttpRequest(req.method, req.url, req.body, {
          params: req.params,
          headers: req.headers,
          context: req.context,
          reportProgress: req.reportProgress,
          responseType: req.responseType,
          withCredentials: req.withCredentials,
        })
      );
    }

    return next.handle(req);
  }
}

Also important to note that this PR doesn't break any functionality, just adds an extra use case.

We made this PR because we're using it internally but we're having to use patch-package to add this in :(

@jerbob92
Copy link
Contributor

jerbob92 commented Jul 9, 2022

Interesting, I'll merge it but keep the warning in the readme since it won't work by default.

@jerbob92 jerbob92 merged commit f3d8206 into klippa-app:master Jul 9, 2022
@edusperoni edusperoni deleted the fix/hxr-httpformdata branch July 9, 2022 12:20
@jerbob92
Copy link
Contributor

jerbob92 commented Jul 9, 2022

This has been released for all NS versions.

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

Successfully merging this pull request may close these issues.

None yet

2 participants