search

ColemanGariety / gulp-nodemon

gulp + nodemon + convenience
526 stars 76 forks source link

Removed event-stream dependency #164

Closed aal89 closed 5 years ago

aal89 commented 5 years ago

What does this PR do?

Locks Removes the event-stream dependency and updates nodemon dependency to 1.18.7 to mitigate a possible attack. Also included the package-lock.json file to have reproducible builds. This most probably helps to stay safer.

Background https://github.com/dominictarr/event-stream/issues/116

Resolves https://github.com/JacksonGariety/gulp-nodemon/issues/163

TwoAbove commented 5 years ago

https://github.com/JacksonGariety/gulp-nodemon/pull/165 This one is better imo

joebowbeer commented 5 years ago

Or update to event-stream@4.0.1 (a la #165 ), which is also clean of the bogus flatmap-stream dep. Better yet, by the burn-me-once principle: move off of event-stream?

yoelfme commented 5 years ago

Or update to event-stream@4.0.1 (a la #165 ), which is also clean of the bogus flatmap-stream dep. Better yet, by the burn-me-once principle: move off of event-stream?

yep, that sounds better

aal89 commented 5 years ago

I have updated the PR. Ever since @joebowbeer commented: 'burn-me-once principle: move off of event-stream', I checked if it was used anywhere and ran the test, but couldn't find any problems after removing it entirely. Could people verify? Then I think this is the best fix (reference)? Don't require it if you don't need it.

simison commented 5 years ago

This looks great now, @aal89 👍

Indeed looks like event-stream isn't used directly by gulp-nodemon so it's safe to remove.

simison commented 5 years ago

@JacksonGariety hope you're not very busy — would be fantastic to get this change in and new version bumped in NPM.

Thanks for all your work! ❤️

rxmarbles commented 5 years ago

I was curious why event-stream is a dependency, glad this is being removed in this PR. 👍

aal89 commented 5 years ago

@rkmarks I looked through the history and it seems that it once had a purpose, however somewhere down the road it had been removed, just never as a dependency.

map-stream was being replaced by event-stream here and actually using a tilde for version selection. It later got updated and changed to a carot for version selection here. Continuing down the road it got updated, eventually requiring the dirty version. (disc: not implying the tilde/carot thing is bad).

yoelfme commented 5 years ago

that's all @aal89, thanks for your help. @JacksonGariety woulb be great if we can get these changes in a new version