Skip to content
This repository has been archived by the owner on Jan 28, 2022. It is now read-only.

Call gulp with npx #42

Merged
merged 1 commit into from
Feb 7, 2019
Merged

Call gulp with npx #42

merged 1 commit into from
Feb 7, 2019

Conversation

matejlauko
Copy link
Contributor

This will find gulp in mono-repo!
This package has gulp in dependencies so it creates a binary in the main node_modules/.bin
Problem in mono-repo was that webpack-deploy got installed to specific app's note_modules and gulp to the main/root node_modules so it couldn't have been called easily. This was solved by listing gulp as direct dependency, but that's not a good solution.

This solves it I think, because npx can find it recursively. Didn't want to use yarn, because then we would need to make that a requirement to use webpack-deploy. So now it's rather dependant on npm >= 5.2.0 because that's when npx got first bundled with npm

This will find gulp in mono-repo
@matejlauko matejlauko self-assigned this Feb 5, 2019
@matejlauko matejlauko changed the title Call gulp with npx RFC: Call gulp with npx Feb 5, 2019
@jukben
Copy link
Contributor

jukben commented Feb 5, 2019

Makes sense. Let's remove gulp from dependencies then, right? Or at least move it to the devDependencies. Does it make sense?

@LeZuse LeZuse added the RFC label Feb 6, 2019
Copy link
Member

@LeZuse LeZuse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening up this topic and for a PR! 🙏

I think it would be great to revisit the original project assumption https://github.com/productboard/webpack-deploy/blob/master/README.md#why-bash-with-gulp

IMO, the landscape changed significantly in the last 4 years and it would maybe make sense to deprecate gulp altogether. I would definitely be up for something like https://github.com/oclif/oclif

Nevertheless, I definitely understand the pain point and agree to hot fix it for now.
As long as the change doesn't break any current usage npx looks like a nice solution for that.

@jukben You still need the gulp package in runtime so I am not sure devDeps is the right way to go. Is peerDependencies the right solution here?

@jukben
Copy link
Contributor

jukben commented Feb 6, 2019

@LeZuse Oh my! That oclif looks awesome! ❤️ Definitely looks like something we can use for this. Totally agree that would be the best to deprecate gulp entirely. On the other hand, I know that there are different priorities.

Ah you are correct – devDependencies would do the trick. On the other hand in this state, it's probably doesn't matter. The most important is the fact that this fixes some issue – if so, and I believe so, let's merge it. 🚢

@matejlauko
Copy link
Contributor Author

@jukben @LeZuse
Gulp actually needs to be in dependencies! I'm removing it from PB's dependencies, so this package needs to install gulp.

Didn't want peerDependencies, because we would still need to explicitly list gulp in PB

@matejlauko matejlauko merged commit 5df43b9 into master Feb 7, 2019
@matejlauko matejlauko deleted the gulp-npx branch February 7, 2019 17:57
@matejlauko matejlauko changed the title RFC: Call gulp with npx Call gulp with npx Feb 7, 2019
@tozes tozes self-requested a review February 8, 2019 11:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants