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

Fixing an issue that type 'null' and 'undefined' is not assignable to validateStatus when typescript strict option is enabled #3200

Merged
merged 1 commit into from Oct 20, 2020

Conversation

@cprayer
Copy link
Contributor

@cprayer cprayer commented Aug 14, 2020

In previous my PR(#2773), I had missed about two things.

  1. Assign function type (one status number parameter, return boolean or null type) instead of function type(one status number parameter, return boolean type), null and undefined type
  2. Doesn't care about undefined type

So I fix above things in this PR.

Copy link
Collaborator

@chinesedfan chinesedfan left a comment

Thanks for your update. And in order to avoid similar mistakes, can you update the test file with a new case? In addition, validateStatus? has included undefined I think.

@chinesedfan
Copy link
Collaborator

@chinesedfan chinesedfan commented Oct 7, 2020

@cprayer Ping for updating.

… validateStatus when typescript strict option is enabled
@cprayer cprayer force-pushed the cprayer:fix-type-error branch from fe85e35 to 89c40f3 Oct 14, 2020
@cprayer
Copy link
Contributor Author

@cprayer cprayer commented Oct 14, 2020

@chinesedfan
Thanks your review! I have missed about this PR.
As you said, I change the ValidStatus option to ((status: number) => boolean) | null and add some test case about null and undefined.

Before -> validateStatus?: ((status: number) => boolean | null)

tsc test/typescript/axios.ts --target es6 --moduleResolution node --strict
test/typescript/axios.ts:48:3 - error TS2322: Type 'null' is not assignable to type '((status: number) => boolean) | undefined'.

48   validateStatus: null
     ~~~~~~~~~~~~~~

  index.d.ts:65:3
    65   **validateStatus?: ((status: number) => boolean) | null**
         ~~~~~~~~~~~~~~
    The expected type comes from property 'validateStatus' which is declared here on type 'AxiosRequestConfig'

test/typescript/axios.ts:341:9 - error TS2345: Argument of type '(value: string) => void' is not assignable to parameter of type '(value: string | AxiosResponse<any>) => void | PromiseLike<void>'.
  Types of parameters 'value' and 'value' are incompatible.
    Type 'string | AxiosResponse<any>' is not assignable to type 'string'.
      Type 'AxiosResponse<any>' is not assignable to type 'string'.

341   .then((value: string) => {});
            ~~~~~~~~~~~~~~~~~~~~~

test/typescript/axios.ts:345:9 - error TS2345: Argument of type '(value: string) => void' is not assignable to parameter of type '(value: string | AxiosResponse<any>) => void | PromiseLike<void>'.
  Types of parameters 'value' and 'value' are incompatible.
    Type 'string | AxiosResponse<any>' is not assignable to type 'string'.
      Type 'AxiosResponse<any>' is not assignable to type 'string'.

345   .then((value: string) => {});
            ~~~~~~~~~~~~~~~~~~~~~

After - validateStatus?: ((status: number) => boolean) | null

tsc test/typescript/axios.ts --target es6 --moduleResolution node --strict
test/typescript/axios.ts:341:9 - error TS2345: Argument of type '(value: string) => void' is not assignable to parameter of type '(value: string | AxiosResponse<any>) => void | PromiseLike<void>'.
  Types of parameters 'value' and 'value' are incompatible.
    Type 'string | AxiosResponse<any>' is not assignable to type 'string'.
      Type 'AxiosResponse<any>' is not assignable to type 'string'.

341   .then((value: string) => {});
            ~~~~~~~~~~~~~~~~~~~~~

test/typescript/axios.ts:345:9 - error TS2345: Argument of type '(value: string) => void' is not assignable to parameter of type '(value: string | AxiosResponse<any>) => void | PromiseLike<void>'.
  Types of parameters 'value' and 'value' are incompatible.
    Type 'string | AxiosResponse<any>' is not assignable to type 'string'.
      Type 'AxiosResponse<any>' is not assignable to type 'string'.

345   .then((value: string) => {});
            ~~~~~~~~~~~~~~~~~~~~~

TS2345 Error of line 341, 345 is irrelevant to the validateStatus option. So I don't care about this.

@cprayer cprayer requested a review from chinesedfan Oct 14, 2020
@chinesedfan chinesedfan added the v0.20.1 label Oct 17, 2020
@chinesedfan chinesedfan requested a review from jasonsaayman Oct 17, 2020
Copy link
Collaborator

@jasonsaayman jasonsaayman left a comment

This seems to be a good solution to this issue.

@jasonsaayman jasonsaayman merged commit 3a8b87d into axios:master Oct 20, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.