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

Bumps @svgr/webpack dependency to version 6.2.1 #12172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

daschaa
Copy link

@daschaa daschaa commented Mar 15, 2022

Regarding the issue #12146 the @svgr/webpack dependency has to be updated to fix the security warning related to the transitive nth-check dependency.

@daschaa
Copy link
Author

daschaa commented Apr 5, 2022

@iansu / @mrmckeb
Any feedback on this? Would be cool to have this merged so repositories which are using CRA (react-scripts in particular) would not be bothered by dependabot complaining about the CVE (GHSA) in nth-check.

@bbodensieck
Copy link

All checks are fine. Could you please merge this?

@jy95
Copy link

jy95 commented May 31, 2022

Up

@ranman
Copy link

ranman commented Jul 20, 2022

This should be merged.

@bakgaard
Copy link

bakgaard commented Aug 2, 2022

Please do hit the merge-button 🙂

@BrandonKoala
Copy link

@iansu @mrmckeb Bumping this for visibility

@Lsnsh
Copy link

Lsnsh commented Aug 22, 2022

These issues may all need this PR:

Similar PR, no progress:


SVGR issue, about SVG with CDATA:

@daschaa
Copy link
Author

daschaa commented Aug 23, 2022

Tbh I am also quite surprised that there is not even a response by the maintainers to this pull request.

@seavor
Copy link

seavor commented Aug 31, 2022

"Please sir, may I have some merges?"

1_WoJk_ozkGZ9fYzvW0MriKw

@jy95
Copy link

jy95 commented Aug 31, 2022

image

@Master-Guy
Copy link

Would it help to @ the two reviewers?

@rap2hpoutre
Copy link

@Master-Guy You are right, we can try @mrmckeb @iansu 👋

@szakharchenko
Copy link

Tbh I am also quite surprised that there is not even a response by the maintainers to this pull request.

Only 6 months? Those are rookie numbers.

@daschaa
Copy link
Author

daschaa commented Sep 21, 2022

@gaearon Can we close this pull request? Or is it maybe a good idea to bump the dependency to a newer version even if the vulnerability is not affecting react-scripts?
I don't know the details of the newest version here, but maybe there are internal improvements as well.

@sebastienpa
Copy link

@iansu @mrmckeb please merge 🙏

1 similar comment
@mpavlikWandera
Copy link

@iansu @mrmckeb please merge 🙏

@wozzo
Copy link

wozzo commented Nov 9, 2022

Tbh I am also quite surprised that there is not even a response by the maintainers to this pull request.

Only 6 months? Those are rookie numbers.

What do I win? 🤣

@vijaya-lakshmi-venkatraman

Hi,
Is there a timeline to expect this merge so we can plan accordingly?

@Phonesis
Copy link

Quite vital this is merged soon as it a security issue

@mpavlikWandera
Copy link

This may not be a security issue in itself, but it is blocking us from fixing other security issues because this is a blocking dependency :( .

@alexishecf
Copy link

Not impacting production but pretty straightforward, should be merged

@andresmanikis
Copy link

andresmanikis commented Dec 2, 2022

You can always override the dependency like this in your project's package.json.

"overrides": {
    "react-scripts": {
      "@svgr/webpack": "6.5.1"
    }
  }

Having said that, I think this should be reviewed by the maintainers. Don't know why it hasn't been yet.

@andresmanikis
Copy link

Hi @daschaa, I'm glad you took the time to raise this PR.
I'm not sure which should be the test plan to make sure everything is working properly after the change.
I guess at least checking that SVGs are loaded correctly and also all the tests are passing (which I guess must be part of the CI checks, right?).

Would be great if it can be reviewed by one of the owners and see if there's something else needed.

@danvitoriano
Copy link

Hi people, any updates on merge this PR? My Sec team is hopefully waiting for us to fix the Snyk vulnerability on this svg lib version <3

@daschaa
Copy link
Author

daschaa commented Dec 8, 2022

@andresmanikis
Thanks for your feedback. I will add all the necessary stuff as soon as a maintainer who is willing to review this PR gives feedback.
I don't know why no one from Meta looks at this. At least any kind of feedback would be nice.

@andresmanikis
Copy link

Yes. Don't know either.

@mpavlikWandera
Copy link

@iansu @mrmckeb could you pls have a look? or at least give us a reason why this is stuck for so long?

dbussink added a commit to planetscale/vitess that referenced this pull request Feb 8, 2023
This includes the override of the upgrade proposed in
facebook/create-react-app#12172.

I think though that we really should look at moving away from
`create-react-app` since PRs like that and other issues show it's not
really maintained anymore.

We could switch to something like Vite instead but this is a quicker way
for now to silence Dependabot.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
lauramosher added a commit to Terrastories/explore-terrastories that referenced this pull request Feb 27, 2023
This also adds an explicit override to react-scripts due to a
vulnerability in a transient package.

See issue in [create-react-app](facebook/create-react-app#11174)
and linked open PR to bump the dependency [here](facebook/create-react-app#12172).

For now, we are adding our override to bump the version of webpack.
@tgross35
Copy link

tgross35 commented May 30, 2023

There are a few sources suggesting this tool is deprecated (e.g. #13072), which seems to line up with the lack of maintenance. It seems like that might be the reason this hasn't been addressed in way too long.

Edit: better source reactjs/react.dev#5487

@ethhandy
Copy link

ethhandy commented Sep 2, 2024

You can always override the dependency like this in your project's package.json.

"overrides": {
    "react-scripts": {
      "@svgr/webpack": "6.5.1"
    }
  }

Having said that, I think this should be reviewed by the maintainers. Don't know why it hasn't been yet.

@andresmanikis, this doesn't work for old versions of npm.
My npm version is 6.14.15 and node version is 14.18.3. So, can't use override.

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

Successfully merging this pull request may close these issues.