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

AxiosError should be a top-level export instead of a static #5062

Open
GuichiZhao opened this issue Oct 8, 2022 · 11 comments
Open

AxiosError should be a top-level export instead of a static #5062

GuichiZhao opened this issue Oct 8, 2022 · 11 comments

Comments

@GuichiZhao
Copy link

GuichiZhao commented Oct 8, 2022

Describe the bug

There is no way to use AxiosError correctly (It should be used as both type and value)

To Reproduce

import { AxiosError as AxiosErrorFromImport } from 'axios';
import axios from 'axios';

/**
 * AxiosError is resolved as a Class from the "import"
 * Class can be used as both type and value in typescript
 * Such structure should be the right way to do types
 * The following code has no compile time error
 *
 * BUT it has a runtime error ERROR because the Implementation
 * is not export, which is a disaster
 */
const errorHandler = (error: AxiosErrorFromImport) => {
  console.log(AxiosErrorFromImport.ERR_BAD_OPTION);
  console.log(error.response);
};

/**
 * Actually, the implementation is added to the default export as static
 */
const { AxiosError } = axios;

/**
 * By doing this AxiosError is not treated as a class ANYMORE
 * so the following code has a compile time error
 */

// 'AxiosError' refers to a value, but is being used as a type here
// Did you mean 'typeof AxiosError'?
const errorHandler1 = (error: AxiosError) => {};

// Event though I follow the error prompts
const errorHandler2 = (error: typeof AxiosError) => {
  // Property 'config' does not exist on type 'typeof AxiosError'.
  error.config
};
// There is no way to use the type declaration


// So, what is the point to put AxiosError as a static?
// It totally screwed up the type declaration:
// AxiosError can be used as a type by importing from top-level
// AxiosError can be used as a value by geting it from default import
// It is very confusiong and inconvenient 


Expected behavior

Put AxiosError as top-level export with implementation rather than type ONLY

Environment

  • Axios 1.1.2
  • Typescript 4.6.3
@haneenmahd
Copy link

I am opening a PR on this 👍 !

@chrisweb
Copy link

chrisweb commented Oct 8, 2022

This might be related to #5031

@GuichiZhao
Copy link
Author

I am opening a PR on this +1 !

The problem is not name conflicting, it is the wrong way (not typescript wise) of exporting

@haneenmahd
Copy link

Okay so I should change the way it is exported, right?

@GuichiZhao
Copy link
Author

Okay so I should change the way it is exported, right?

In my opinion, the whole class (both type and implementation) should be exported at top level. Currently only type (interface) is exported at top level, the real implementation is hidden deep inside, which is the root of the trouble

haneenmahd added a commit to haneenmahd/axios that referenced this issue Oct 9, 2022
Currently, there is no way to use AxiosError correctly. This was caused only by exporting the type at the top-level but the implementation deep inside the axios instance.
Exported the AxiosError class at the top-level to fix inconsistencies.

Refs: axios#5062

BREAKING CHANGE: AxiosError should be used from the top-level import rather than using it from the axios instance.
@haneenmahd
Copy link

I have fixed it 👍🏻 !

Can you take a look at it?

@GuichiZhao
Copy link
Author

I have fixed it 👍🏻 !

Can you take a look at it?

I am not maintainer of axios and do not want to involve too much, you can explain the reason and ask someone relevant to do the reviewing.

@jasonsaayman
Copy link
Member

Will have a look @haneenmahd

@littledian
Copy link
Contributor

It seems pr: #5030 fixed it but then commit: #2149464bb4f12c1101b15f73298a060e92470376 revert it

@Erazihel
Copy link

I tried to update axios to v1.1.3 but the error is still here unfortunately.

const axiosError = new AxiosError();
TypeError: _axios.AxiosError is not a constructor

@littledian
Copy link
Contributor

I tried it after upgrade axios to v1.1.3, It works when I import it but not work when require it;
eg:
It works: import axios from 'axios' or import { AxiosError } from 'axios';
It not works in broswer: const axios = require('axios'), but it works in nodejs env

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants