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

Parallel taks are forwarded to the end command #26

Closed
tleunen opened this issue Jun 27, 2016 · 9 comments
Closed

Parallel taks are forwarded to the end command #26

tleunen opened this issue Jun 27, 2016 · 9 comments

Comments

@tleunen
Copy link
Collaborator

tleunen commented Jun 27, 2016

ERROR in multi main
Module not found: Error: Cannot resolve module 'build.test,build.staging,build.edge'

  • p-s version: 1.0.2
  • node version: 6.6.1

The command executed:

npm start -p build.x,build.y

The output:

ERROR in multi main
Module not found: Error: Cannot resolve module 'build.x,build.y'

Problem description:
My build command run webpack to build a bundle file for my project. But when running npm start -p build.x,build.y, it appends the parallel commands to webpack and produce an error:

webpack 'build.x,build.y'

I think the parallel taks shouldn't be forwarded to the end command

@kentcdodds
Copy link
Collaborator

Thanks for the report!

Hmmm... Very interesting. If you'd like to dig, here's where we exclude p-s flags from the args that are forwarded on to the scripts.

@tleunen
Copy link
Collaborator Author

tleunen commented Jun 27, 2016

Thanks, I'll take a look :)

@tleunen
Copy link
Collaborator Author

tleunen commented Jun 28, 2016

@kentcdodds When running npm start -p this,that. program.parallel is undefined in getScriptsAndArgs.

@tleunen
Copy link
Collaborator Author

tleunen commented Jun 28, 2016

Oh I see...

npm start -p this,that is executing p-s "this,that".. So the parallel option is gone. I guess npm takes it as an option instead of p-s :/

So I'm now using npm start -- -p this,that which resolves the issue.

Closing the ticket, unless you know of a "fix"?

@tleunen tleunen closed this as completed Jun 28, 2016
@kentcdodds
Copy link
Collaborator

Ah, yep, that's the solution. Do you think we could/should add something to the docs as a common pitfall?

@tleunen
Copy link
Collaborator Author

tleunen commented Jun 29, 2016

Might be useful to add a note for it when using npm start instead of p-s. In my case, I'm using npm start because it's part of a build system. And using p-s is not possible. Using ./node-modules/.bin/p-s fails to run sub-commands because p-s is not found.

@kentcdodds
Copy link
Collaborator

Might be useful to add a note for it when using npm start instead of p-s. In my case, I'm using npm start because it's part of a build system

Actually, the recommended use is to use npm start, but to put the scripts to be run in package-scripts.json. So generally you'd put the script you're trying to run in the package-scripts.json like this. Also, I think the thing that caused confusion is a fundamental misunderstanding of how npm scripts work. So maybe documenting something to remind people that if you want to make sure to pass arguments to a script, you need to use --. That's what I was thinking.

And using p-s is not possible. Using ./node-modules/.bin/p-s fails to run sub-commands because p-s is not found.

I think you have a typo. node-modules should be node_modules. You should be able to run node_modules/.bin/p-s. But really, you should be just using p-s in an npm script and it will work fine.

@tleunen
Copy link
Collaborator Author

tleunen commented Jun 29, 2016

Oh I see, you mean having a simple script which will run my tasks in parallel. Yep I could do that too.

And yes, sorry for the typo. What I mean though was that if I manually run ./node_modules/.bin/p-s this and then this also run p-s that, the second command fails because p-s is not available. But that's expected I guess ;)

@kentcdodds
Copy link
Collaborator

Ah, I see what you're saying. Yeah, basically the way that p-s works is it spawns a script with the same process.env as the current environment. So if the currently PATH doesn't include node_modules/.bin then the spawned script wont have it either. I suppose that p-s could explicitly add node_modules/.bin to the PATH. I'll create an issue for discussion about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants