search

jonkemp / inline-css

Inline css into an html file.
MIT License
429 stars 85 forks source link

Runs npm audit and adds missing test packages #106

Closed ryanlabouve closed 2 years ago

ryanlabouve commented 2 years ago

Tried to use this project and noticed some security vulns.

Sorry for the massive diff, but this is mostly NPM code.

ryanlabouve commented 2 years ago

Noticed no CI, so adding a screenshot of the test

-bash 2021-09-25 10-56-34

Please let me know if you'd like me to add a testing workflow via Github actions :-D

ryanlabouve commented 2 years ago

Good talk

jonkemp commented 2 years ago

Sorry @ryanlabouve. You're right. I should have at least left a comment explaining why this was closed.

I closed it because I felt it wasn't really needed. Running audit fix just updates the package-lock.json file which I just don't feel is necessary for someone to submit a pr for. Just my opinion. I also deleted the node modules and then ran npm install again and didn't see any missing packages so I didn't feel that was necessary. In addition, lerna was updated but extract-css was updated to an older version. That kind of change should have an explanation and I don't see a reason for it.

I appreciate your wishing to contribute but I'm working on a significant maintenance pr which I usually do myself right now. Please feel free to share anymore ideas you have for further contributions.

ryanlabouve commented 2 years ago

Thanks for the response.

Tests were failing when I ran locally. You may have some packages installed globally that I added.

And there are security issues. Updates in the package.json resolves these.

Just figured I'd leave some possible solutions instead of issues.

On Oct 15, 2021, at 9:25 PM, Jonathan Kemp @.***> wrote:

 Sorry @ryanlabouve. You're right. I should have at least left a comment explaining why this was closed.

I closed it because I felt it wasn't really needed. Running audit fix just updates the package-lock.json file which I just don't feel is necessary for someone to submit a pr for. Just my opinion. I also deleted the node modules and then ran npm install again and didn't see any missing packages so I didn't feel that was necessary. In addition, lerna was updated but extract-css was updated to an older version. That kind of change should have an explanation and I don't see a reason for it.

I appreciate your wishing to contribute but I'm working on a significant maintenance pr which I usually do myself right now. Please feel free to share anymore ideas you have for further contributions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

alumni commented 2 years ago

@jonkemp not all of us are using npm, and yes, we could manually add package resolutions in package managers, but this won't fix the issues that come from the old cheerio version. Maybe you could set up Dependabot/WhiteSource Renovate/Snyk for this repo?