Skip to content

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

Merged
jasonsaayman merged 1 commit intoaxios:masterfrom
cprayer:fix-type-error
Oct 20, 2020
Merged

Conversation

@cprayer
Copy link
Copy Markdown
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.

chinesedfan
chinesedfan suggested changes Aug 15, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chinesedfan chinesedfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy Markdown
Contributor

chinesedfan commented Oct 7, 2020

@cprayer Ping for updating.

@cprayer
Fixing an issue that type 'null' and 'undefined' is not assignable to…
89c40f3 
… validateStatus when typescript strict option is enabled
@cprayer
Copy link
Copy Markdown
Contributor Author

cprayer commented Oct 14, 2020 •
edited
Loading

@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 October 14, 2020 16:55
jasonsaayman
jasonsaayman approved these changes Oct 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jasonsaayman jasonsaayman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a good solution to this issue.

@jasonsaayman jasonsaayman merged commit 3a8b87d into axios:master Oct 20, 2020
This was referenced Jan 5, 2021
Open
Open
Closed
Merged
Merged
This was referenced Mar 5, 2021
Merged
Open
Open
Merged
Open
Merged
Closed
Closed
Open
Open
Merged
Open
Merged
This was referenced Mar 18, 2021
Open
Open
Merged
Open
Closed
Open
Closed
Merged
Open
Open
Closed
Open
Merged
Merged
Open
Merged
Merged
Open
Merged
Merged
Merged
Merged
Open
@sync-by-unito sync-by-unito Bot mentioned this pull request Jun 28, 2021
Closed
mbargiel pushed a commit to mbargiel/axios that referenced this pull request Jan 27, 2022
@cprayer
Fixing an issue that type 'null' and 'undefined' is not assignable to…
2fe61b5 
… validateStatus when typescript strict option is enabled (axios#3200)
@Omrisnyk Omrisnyk mentioned this pull request Dec 20, 2023
Open
@Omrisnyk Omrisnyk mentioned this pull request Jan 2, 2024
Open
@Omrisnyk Omrisnyk mentioned this pull request Mar 16, 2024
Open
This was referenced Oct 27, 2025
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonsaayman jasonsaayman jasonsaayman approved these changes

+1 more reviewer

@chinesedfan chinesedfan chinesedfan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cprayer @chinesedfan @jasonsaayman