ngneat / cashew

🐿 A flexible and straightforward library that caches HTTP requests in Angular
https://www.netbasal.com
MIT License
682 stars 33 forks source link

HttpParams not being parsed on http Post correctly #32

Closed chris480 closed 3 years ago

chris480 commented 3 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ X] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

When doing a http POST with HttpParams calls wrong url.

The called url is https://localhost:4200/api/search?updates=%5Bobject%20Object%5D&cloneFrom=&encoder=%5Bobject%20Object%5D&map=null

Expected behavior

Can pass a variable with HttpParams type into WithCache and correct url is called. HTTP call functions as normal. Cache is stored as per cashew localstorage.

Minimal reproduction of the problem with instructions

Given a this test code.

    ...
    params = new HttpParams();
    params = params.append('context', context)

    return new Promise((resolve, reject) => {
      this.http.post(url, payload, withCache({...params})).subscribe((result) => {
        resolve(result);
      }, (error) => {
        reject(error);
      });
    });

Cashew clearly tries to convert HttpParams to query string. Is there a specific object I should be passing?

Environment


Angular version: X.Y.Z
8.2.3

Browser:
- [X ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: XX  
- Platform:  

Others:
Using Local Storage
NetanelBasal commented 3 years ago

Can you please reproduce it on stackblitz?

chris480 commented 3 years ago

Replicated on SB: https://stackblitz.com/edit/angular-ivy-n6csyb?file=src%2Fapp%2Fhello.component.ts Also occuring on A11

Using jsonplaceholder as a fake api service

NetanelBasal commented 3 years ago

Why you're not using a pojo?

    const params = {
      api_key: "1234",
      url: "https://google.com/"
    };

    this.http
      .get(
        "https://jsonplaceholder.typicode.com/todos/",
        withCache({ ...params })
      )
chris480 commented 3 years ago

The HttpParam object is an output from another component. All the services were given HttpParams by default. If cashew doesn't have a native way to convert them, I can just create a function to change httpparam to an object.

Just wanted to check if I was missing something in cashew. If not, it's all good and we can close this out.

NetanelBasal commented 3 years ago

I don't mind getting a PR that adds built-in support for it. If you can't, in the meantime, you can use a function to convert it.

kauppfbi commented 3 years ago

I also had some issues today when I used the library the first time. I wanted to create a PR, but when I further investigated the issue, I found the solution to this problem (which is inside the application code) :

const  params = new HttpParams();
params = params.append('context', context)

// w/o cashew
// const options = {params};

// example above ("wrong approach"):
// const options = withCache({...params});

// with cashew ("right approach"):
const options = withCache({params});

this.http.get(url, options).subscribe();

I also adjusted the Stackblitz example, which is now working as expected: https://stackblitz.com/edit/angular-ivy-erpgym?devtoolsheight=33&file=src/app/hello.component.ts

kauppfbi commented 3 years ago

Okay, nevermind my comment before. Coming back to work topics, I need to see, that it is still not working correctly.

I recognized, that the request url to the api is not built correctly. This is what happens: It builds a url in the format like this: <baseUrl>/api/v1/items?params=foo=bar&bar=foo

I don't know why, but it appends an additional params=. Without, everything would work fine

sysmat commented 3 years ago
export declare function withCache(params?: Params): {
    params: Params;
};
get<T>(url: string, options?: {
        headers?: HttpHeaders | {
            [header: string]: string | string[];
        };
        observe?: 'body';
        params?: HttpParams | {
            [param: string]: string | string[];
        };
        reportProgress?: boolean;
        responseType?: 'json';
        withCredentials?: boolean;
    }): Observable<T>;
NetanelBasal commented 3 years ago

The library now uses context.