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

Separate CLI and nodeAPI #94

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Separate CLI and nodeAPI #94

merged 1 commit into from
Jan 8, 2018

Conversation

tommytroylin
Copy link
Contributor

In our project we use webpack to bundle all js files into a single one.

The first issue is that there is #! /usr/bin/env node at the top of index.js. It will cause an error when webpack tries to resolve this module. And This issue is solved by adding a loader to remove that piece of code.
The second is Error: rc(name): name *must* be a string

When we actually running the bundled code. We got that error. After digging the source code, we find errors happened here

It seems like when this module is running from cli, it will output something to console.
But in webpack bundle, module.parent is not implemented.
And proccess.argv[2] is undefined in this case.

Now we simply remove that console.log and everything works fine.

This PR solves these 2 issues together.

@dominictarr
Copy link
Owner

that is a bug in webpack, webpack/webpack#2168

@tommytroylin
Copy link
Contributor Author

@dominictarr
I understand.
The #! issue is a webpack bug and we actually fixed that by adding a new loader.

But the second issue is a bit annoying, and I hope it can be fixed in the next release since we need to modify the source code manually now.

Thank you in advance.

@chabou
Copy link

chabou commented Oct 11, 2017

Please merge this PR 🙏

First issue can be fixed with this webpack workaround (using shebang-loader):

{
  test: /index.js/,
  loader: 'shebang-loader',
  include: [/node_modules\/rc/]
}

But we can't do anything for this console output 😞

I really need this PR to be merged to use npm-name in a webpacked cli tool.

@chabou chabou mentioned this pull request Oct 18, 2017
15 tasks
@rauchg
Copy link

rauchg commented Oct 19, 2017

This PR makes a lot of sense! I would merge it.

@chabou
Copy link

chabou commented Jan 4, 2018

@dominictarr Is there a chance to have this PR merged soon? 🙏

@dominictarr dominictarr merged commit 1037247 into dominictarr:master Jan 8, 2018
@dominictarr
Copy link
Owner

reluctantly merged this into 1.2.3
once upon a time this just worked in browserify, but now neither browserify or webpack support this by default so people end up posting issues all the time!

@chabou
Copy link

chabou commented Jan 8, 2018

Thank you so much!

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

Successfully merging this pull request may close these issues.

4 participants