PalisadoesFoundation / talawa-admin

Admin portal for the Talawa Mobile App. Click on the link below to see our documentation
https://docs.talawa.io/
GNU General Public License v3.0
158 stars 653 forks source link

Resolve dependency conflicts #453

Closed palisadoes closed 1 year ago

palisadoes commented 1 year ago

Describe the bug

  1. Run the commands
            npm i --package-lock-only
            npm audit
  2. The output shows a large number of dependency conflicts

To Reproduce See above

Expected behavior

  1. These conflicts need to be resolved.
  2. Update the GitHub action for pull-requests so that they fail when future dependency conflicts are found.

Actual behavior

See above

Screenshots This is sample output from the npm i --package-lock-only command image

Additional details See above

palisadoes commented 1 year ago

@xoldyckk @noman2002

  1. Is this issue worthwhile to pursue?
  2. We also have a large number of vulnerabilities related to react-scripts and I'd like to know whether we should consider removing it from the application. Should we?
noman2002 commented 1 year ago

I read somewhere that these warnings arise due to conflicts between the packages we used. I don't know if it possible to remove all of them or not.

palisadoes commented 1 year ago

@noman2002 What about react-scripts? We have close to a dozen reported vulnerabilities that cannot be updated because the latest version of react-scripts requires vulnerable dependencies. Is there an alternative we could use?

aashimawadhwa commented 1 year ago

is this somewhere related to issue #394 ? @palisadoes

palisadoes commented 1 year ago

@aashimawadhwa

  1. Yes it is. I forgot about that one. There may be a way to run a command to automatically fix the dependencies.
  2. What are your thoughts about react-scripts?
AjayMaheshwari23 commented 1 year ago

@aashimawadhwa i think you are working on #394 , so if you want you can assign this to me so that i can also give it a try

aashimawadhwa commented 1 year ago

@aashimawadhwa

1. Yes it is.  I forgot about that one. There may be a way to run a command to automatically fix the dependencies.

2. What are your thoughts about `react-scripts`?

i tired updating the dependencies automatically and right now i am doing everything manually

palisadoes commented 1 year ago

Hopefully when we get them all OK we can then add the test to GitHub actions so that we never have to go through this again.

aashimawadhwa commented 1 year ago

https://stackoverflow.com/questions/16073603/how-to-update-each-dependency-in-package-json-to-the-latest-version https://classic.yarnpkg.com/lang/en/docs/cli/upgrade-interactive/

aashimawadhwa commented 1 year ago

@palisadoes I am following these resources

aashimawadhwa commented 1 year ago

According to TL;DR: peerDependencies is for dependencies that are exposed to (and expected to be used by) the consuming code, as opposed to "private" dependencies that are not exposed, and are only an implementation detail.

If P1 and P2 indeed both have peer dependencies on incompatible versions of P3 you will need to either find versions of P1 and P2 with compatible peer dependencies of P3 or abandon the use of either P1 or P2. @palisadoes

aashimawadhwa commented 1 year ago

and abandoning dependencies does not feel right to me for some reason, it will break the code to great extent.

aashimawadhwa commented 1 year ago

I updated all the packages in package.JSON and tried everything but the npm warnings still exists. @palisadoes Screenshot 2023-02-09 at 2 39 20 AM (2)(1) Screenshot 2023-02-09 at 2 39 20 AM (2)

palisadoes commented 1 year ago

Thanks for the effort @aashimawadhwa .

  1. How many warnings do we have now?
  2. We are seeing most of our dependency issues related to react-scripts.
    1. Should we continue using it, is it being used?
    2. if so, why?
    3. if not, what alternatives could we use?

The react-scripts repository has not been updated in 10 months on GitHub and 10 months in npm. I'm concerned support may have ended.

aashimawadhwa commented 1 year ago

1.) For warnings there are no changes as of now , if we look closely into them many dependencies are deprecated and therefore the warning is generated. 2.) most of the issues are there because of the react scripts , I am not sure if we should continue using them but we can use the following alternatives for react scripts i.e :

@palisadoes

palisadoes commented 1 year ago

Using deprecated packages is a risk we should not take. How easy would it be to do the replacement?

@noman2002 @xoldyckk what are your thoughts?

noman2002 commented 1 year ago

It it a bit tedious task to change packages as we might not have exact same replacement for it. I think we should keep this as it is for now and update in one of gsoc projects.

palisadoes commented 1 year ago

Thanks Noman,

  1. @aashimawadhwa Please submit a PR for what works so far.
  2. @noman2002 Please update the GSoC security to cover this
xoldd commented 1 year ago

@palisadoes create-react-app is outdated. It focuses on backwards compatiblity when web ecosystem is moving forward at a rapid rate. Modern web technologies don't work with create-react-app and even if they somehow do you'll spend more time configuring create-react-app than working on the business logic. This is because all configuration in create-react-app is abstracted away using react-scripts. Also it's built on webpack which is extremely slow and sucks for developer productivity.

I'd recommend migrating the project to vite with react and typescript.

aashimawadhwa commented 1 year ago

Wdyt Craco migration will help ? @xoldyckk

xoldd commented 1 year ago

@aashimawadhwa It may work but it's a wasted effort. Even when using craco you're still relying on create-react-app indirectly. You're trying to maintain a whole legacy module system based on babel and webpack by yourself. If something in create-react-app's core is broken craco can't fix that.

There's a whole war going on with official react team and big influencers and developers in javascript ecosystem to ban the recommendation of using of create-react-app in react docs.

Look:- https://github.com/reactjs/reactjs.org/pull/5487

And it'll be soon either deprecated or make breaking changes which will not be backwards compatible with older versions of create-react-app. So, either way work has to be done. So, if you're going to do it, do it with what the industry standard is right now.

aashimawadhwa commented 1 year ago

So wdyt updating the yarn.lock and package.json will create some difference?? @palisadoes @xoldyckk

xoldd commented 1 year ago

@aashimawadhwa btw talawa-admin uses an outdated version of yarn too. It uses v1 when current yarn version is v3.

I tried using it with yarn v3 it broke. Idk what's going on but yarn v1 and talawa-admin have some magic going on behind the scenes. And magic is bad.