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

Please update axios #1343

Open
thany opened this issue Feb 16, 2023 · 6 comments
Open

Please update axios #1343

thany opened this issue Feb 16, 2023 · 6 comments
Labels
backlog Issue/PR/discussion is reviewed and added to backlog for the further work

Comments

@thany
Copy link

thany commented Feb 16, 2023

Description

The types returned from AxiosDataFetcher::fetch() from @sitecore-jss/sitecore-jss-nextjs is incompatible with the actual type declared in axios. This happens after updating Axios to the latest version.

Please note that the code runs fine, but won't perform a production build because of a typing error.

JSS returns this type:

export interface AxiosResponse<T = any>  {
  data: T;
  status: number;
  statusText: string;
  headers: any;
  config: AxiosRequestConfig;
  request?: any;
}

Whereas an up-to-date Axios package expects a different, incompatible one:

export interface AxiosResponse<T = any, D = any> {
  data: T;
  status: number;
  statusText: string;
  headers: RawAxiosResponseHeaders | AxiosResponseHeaders;
  config: InternalAxiosRequestConfig<D>;
  request?: any;
}

It bares noting that putting overrides in package.json might work, but doesn't in our case, for some reason. A little bit like this:

{
  "dependencies": {
    "@sitecore-jss/sitecore-jss-nextjs": "^21.0.7",
    "axios": "^1.3.3"
  },
  "overrides": {
    "axios": "$axios"
  }
}

Should work, but doesn't.

Expected behavior

No error, and an up-to-date Axios subdependency in sitecore-jss-nextjs.

Steps To Reproduce

  1. Install the jss-nextjs starter
  2. Update axios to its latest version
  3. Open data-fetcher.ts
  4. Observe the error that VScode or the build process will spew out.

Possible Fix

Please update Axios in @sitecore-jss/sitecore-jss-nextjs at your earliest convenience.

Your Environment

  • Sitecore Version: 9.3
  • JSS Version: 21.0.7
  • Browser Name and version: Firefox 110
  • Operating System and version (desktop or mobile): Windows 10 desktop
  • Link to your project (if available): N/A

Screenshots

image

@art-alexeyenko
Copy link
Contributor

Hi @thany !
First, thank you for reporting this.
Second, a bit of bad news and some context. Updating axios to latest version unfortunately breaks the react app due to this bug: facebook/create-react-app#11889
Regardless, we'll address the issue one way or another: either update axios after react-scripts is updated (hopefully soon), or replace axios usage in package with fetch, to avoid conflicts.

And last, but not least, there are some workarounds you could use. While not an official fix, I hope they can help.

  1. AxiosDataFetcher is only used in Styleguide-Tracking component. If it's not something your nextjs app uses, you can remove lib/data-fetcher and your app should work fine after.
  2. You could bring AxiosDataFetcher into the app remove the import from base package. Version upgrade requires some changes to it, however, this is what it would look like when using v1.x:
    https://github.com/Sitecore/jss/blob/feature/axios-upgrade/packages/sitecore-jss/src/axios-fetcher.ts
    This should help avoid version conflicts too.

@thany
Copy link
Author

thany commented Mar 5, 2023

I have also tried to override the depedency in package.json, from memory I went like this:

"overrides": {
  "axios": "$axios"
}

It worked fine. But strangely this also proves that updating axios can be done without errors, or compiling it with this override in place would undoubtedly reproduce your error. Perhaps because CRA is not a part of a Next.js app, iirc.

@illiakovalenko illiakovalenko added the backlog Issue/PR/discussion is reviewed and added to backlog for the further work label Jul 4, 2023
@xbindt
Copy link

xbindt commented Nov 22, 2023

Can you please look into this because of: An issue discovered in Axios 0.8.1 through 1.5.1 inadvertently reveals the confidential XSRF-TOKEN stored in cookies by including it in the HTTP header X-XSRF-TOKEN for every request made to any host allowing attackers to view sensitive information.

@pzi
Copy link
Contributor

pzi commented Jan 3, 2024

Newer versions of axios would also address the following Security findings:

@jamesryan-dev
Copy link

+1 to upgrade axios

1 similar comment
@chrissnyder2337
Copy link

+1 to upgrade axios

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issue/PR/discussion is reviewed and added to backlog for the further work
Projects
None yet
Development

No branches or pull requests

7 participants