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

Dependencies Cleanup #52

Open
jolanglinais opened this issue Apr 30, 2020 · 8 comments
Open

Dependencies Cleanup #52

jolanglinais opened this issue Apr 30, 2020 · 8 comments
Assignees
Labels
Package: Markdown Editor ⌨️ Type: Dependencies 🏠 Pull requests that update a dependency file

Comments

@jolanglinais
Copy link
Member

Need to take a hard look at all our dependencies.

This impacts security, development workflow (slow builds), and page load times.

@jolanglinais jolanglinais pinned this issue May 4, 2020
@jolanglinais jolanglinais transferred this issue from accordproject/markdown-editor May 13, 2020
@jolanglinais jolanglinais added Help Wanted 🆘 Extra attention is needed Package: Markdown Editor ⌨️ Type: Dependencies 🏠 Pull requests that update a dependency file Type: Enhancement ✨ Improvement to process or efficiency Type: Need Investigation 🔬 labels May 13, 2020
@jolanglinais jolanglinais pinned this issue Jun 3, 2020
@Cronus1007
Copy link
Contributor

@irmerk Shall I work upon this issue.

@Cronus1007
Copy link
Contributor

Cronus1007 commented Mar 10, 2021

@irmerk I have checked the package=markdown-editor and the dependencies that didn't show up are rimraf and npm -run-all.These were stale dependencies and few dependencies like is-url are also missing.

@Cronus1007
Copy link
Contributor

@irmerk So shall I start working upon this issue.

@jolanglinais
Copy link
Member Author

Could you please outline what you plan to change here before opening a PR?

@Cronus1007
Copy link
Contributor

Cronus1007 commented Mar 11, 2021

@irmerk Here the things that I noticed before opening a PR is that I checked each file of the package twice and saw that the package doesn't depend upon the two dependencies like rimraf and npm-run-all.These have their usecase in storybook package only and it is safe to remove these dependencies from the package.

@Hanaffi
Copy link

Hanaffi commented Mar 15, 2021

@irmerk Can you assign this to me?
My approach is to use depcheck to analyze dependencies by using the information provided by npm

@Cronus1007
Copy link
Contributor

Cronus1007 commented Mar 15, 2021

@Hanaffi Good spot.But depcheck has its own limitation. When I ran npx depcheck then I got this.
Screenshot from 2021-03-15 21-51-21

In dev dependecies I agree with only rimraf and npm-run-all
Here dependencies of rollup,stylelint,lint-staged were used but depcheck ignored those since it doesn't check .rc , .config.js etc files. So it can't be relied much. Here I agree that is-url should be added but other dependencies weren't required as it can perform it's function without them as well.

So it will require a manual checkup rather than automated

@jolanglinais
Copy link
Member Author

Sorry for the delay, I think this makes sense @Cronus1007

@jolanglinais jolanglinais removed Help Wanted 🆘 Extra attention is needed Type: Enhancement ✨ Improvement to process or efficiency Type: Need Investigation 🔬 labels Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: Markdown Editor ⌨️ Type: Dependencies 🏠 Pull requests that update a dependency file
Projects
None yet
3 participants