Sitecore / jss

Software development kit for JavaScript developers building web applications with Sitecore Experience Platform
https://jss.sitecore.com
Apache License 2.0
263 stars 275 forks source link

Please update axios #1343

Open thany opened 1 year ago

thany commented 1 year ago

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

Screenshots

image

art-alexeyenko commented 1 year ago

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: https://github.com/facebook/create-react-app/issues/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 commented 1 year ago

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.

xbindt commented 1 year ago

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 commented 10 months ago

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

jamesryan-dev commented 3 months ago

+1 to upgrade axios