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

HttpParams not being parsed on http Post correctly #32

Closed
chris480 opened this issue Mar 25, 2021 · 9 comments
Closed

HttpParams not being parsed on http Post correctly #32

chris480 opened this issue Mar 25, 2021 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@chris480
Copy link

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
Copy link
Member

Can you please reproduce it on stackblitz?

@chris480
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link

kauppfbi commented Apr 6, 2021

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
Copy link

kauppfbi commented Apr 7, 2021

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

@NetanelBasal NetanelBasal added enhancement New feature or request good first issue Good for newcomers labels Apr 7, 2021
@sysmat
Copy link

sysmat commented Apr 12, 2021

  • wrong types in
export declare function withCache(params?: Params): {
    params: Params;
};
  • in http.d.ts
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>;
  • So withCache should return type as http.d.ts, so If I can't add auth headers is not useful
  • I think withCache should return HttpParams
  • using angular 11

@NetanelBasal
Copy link
Member

The library now uses context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants