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

TypeScript 3.7.5 error in the strict mode inside addQueryParams method #22

Closed
genaby opened this issue Mar 2, 2020 · 7 comments
Closed
Assignees
Labels
invalid This doesn't seem right

Comments

@genaby
Copy link
Contributor

genaby commented Mar 2, 2020

private addQueryParams(query: object): string {
    const keys = Object.keys(query);
    return keys.length ? (
      '?' +
      keys.reduce((paramsArray, param) => [
        ...paramsArray,
        param + '=' + encodeURIComponent(query[param])
      ], []).join('&')
    ) : ''
  }

src/templates/client.mustache

(parameter) param: string
No overload matches this call.
Overload 1 of 3, '(callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string, initialValue: string): string', gave the following error.
Type 'string[]' is not assignable to type 'string'.
Overload 2 of 3, '(callbackfn: (previousValue: never[], currentValue: string, currentIndex: number, array: string[]) => never[], initialValue: never[]): never[]', gave the following error.
Type 'string[]' is not assignable to type 'never[]'.
Type 'string' is not assignable to type 'never'.ts(2769)
lib.es5.d.ts(1350, 24): The expected type comes from the return type of this signature.
lib.es5.d.ts(1356, 27): The expected type comes from the return type of this signature.

I suggest following code:

  private addQueryParams(query: {[key:string]:string|number|boolean}): string {
    const keys = Object.keys(query);
    return keys.length === 0 ? ''
      : '?' + keys.map(key => encodeURIComponent(key) + '=' + encodeURIComponent(query[key])).join('&')
  }
@js2me js2me self-assigned this Mar 2, 2020
@js2me js2me added the invalid This doesn't seem right label Mar 2, 2020
@js2me
Copy link
Member

js2me commented Mar 2, 2020

@genaby Hello! Thanks for your issue!
Can you attach your tsconfig.json file for more information ?

@genaby
Copy link
Contributor Author

genaby commented Mar 2, 2020

Hello @js2me

{
    "compilerOptions": {
        "outDir": "./dist/",
        "sourceMap": true,
        "noImplicitAny": true,
        "module": "CommonJS",
        "target": "ES6",
        "jsx": "react",
        "experimentalDecorators": true,
        "removeComments": true,
        "strict": true
    },
    "include": ["./src/**/*"]
}

@genaby
Copy link
Contributor Author

genaby commented Mar 2, 2020

Also I think that array destruction in the reduce is not very effective since each iteration will create new array each time.

@genaby genaby changed the title TypeScript 3.7.5 error in the addQueryParams TypeScript 3.7.5 error in the strict mode inside addQueryParams method Mar 2, 2020
@js2me
Copy link
Member

js2me commented Mar 2, 2020

@genaby Do you want create a PR for this ?

@genaby
Copy link
Contributor Author

genaby commented Mar 2, 2020

sure will do (with small changes to pass all tests)

private addQueryParams(query: {[key:string]:string|string[]|number|number[]|boolean}): string {
    const keys = Object.keys(query);
    return keys.length === 0 ? ''
      : '?' + keys.map(key => encodeURIComponent(key) + '=' + encodeURIComponent(query[key])).join('&')
  }

@js2me
Copy link
Member

js2me commented Mar 2, 2020

@genaby as this is Api module which do requests to the server. I think needs to do more flexible solution for developers usage.
input type queries sometimes will have { optionalKey?: string } and it means that query type should be Record<string, any> or Record<string, string|string[]|number|number[]|boolean|undefined> 🤔

Also undefined values probably should be excluded from resulted query string

@js2me js2me assigned genaby and unassigned js2me Mar 2, 2020
genaby pushed a commit to genaby/swagger-typescript-api that referenced this issue Mar 2, 2020
@js2me js2me mentioned this issue Mar 2, 2020
@js2me
Copy link
Member

js2me commented Mar 2, 2020

@genaby thanks for your contribution :)
Release 1.4.1 at now have this fix

@js2me js2me closed this as completed Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants