shelljs seems NOT compatible with nexe under CentOS 6.5

[freemine]

Node version (or tell us if you're using electron or some other framework):

node: v8.2.1
npm: 5.3.0
nexe: 1.1.3

ShellJS version (the most recent version/Github branch you see the bug on):

shelljs@0.7.8 (npm install shelljs)

Operating system:

CentOS 6.5
uname -a: Linux localhost.localdomain 2.6.32-431.el6.x86_64 #1 SMP Fri Nov 22 03:15:09 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

Description of the bug:

gcc --version: gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)
using nexe to build a binary from js file which require('shelljs'), the binary could be built.
but when the binary is run, it fails with:
module.js:487
throw err;
^

Error: Cannot find module './src/cat'
at Function.Module._resolveFilename (module.js:485:15)
at Function.Module._load (module.js:437:25)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at s ([eval]:1:114)
at [eval]:1:305
at [eval]:3312:3
at Array.forEach (native)
at Object.16../commands ([eval]:3311:23)
at s ([eval]:1:254)

test.js as simple as such:
let shelljs = require('shelljs');
console.log('d');

but, when I run: node ./test.js, it runs smoothly with no error.
but, when i run: ./node_modules/shelljs/bin/shjs ./test, it runs correctly.

Example ShellJS command to reproduce the error:

[freemine]

forget about ./node_modules/shelljs/bin/shjs ./test.
it seems shjs loads ./test.js at runtime by default if missing-extension

[nfischer]

Please provide a complete minimal setup (i.e. full contents of minimal code files, full configuration necessary, and a sequence of commands to run to produce the bug).

I have no experience with nexe, so I get this bug:

> nexe

....> ERROR: trying to use package.json variables, but not setup to do so!

[freemine]

nexe -i pwd/test.js -o pwd/test

[nfischer]

@freemine Again, please provide contents of all code files (you can format them with markdown code blocks). And please provide a complete list of commands you run (i.e. the command to run nexe, then command to execute the script, and any error output the commands produce).

I tried using nexe but it kicked off a massive compilation, which doesn't sound like the correct behavior for a tiny script.

[freemine]

sorry for inconvenience.

I tried using nexe but it kicked off a massive compilation, which doesn't sound like the correct behavior for a tiny script.

This is by-default nexe's behavior, for nexe is going to build nodejs code into platform-dependent executables.
For the very first time you build with nexe, it'll take a long time to build node as well. This
is why you see a buch of compilation. Just let it go and see. BTW, it took 1 hour for me to compile.

Again, below is the only file I demo this error.

All versions:

CentOS 6.5
node: v8.2.1
npm: 5.3.0
nexe: 1.1.3
gcc --version: gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)

test.js as:

const sjs = require('shelljs');
console.log('good');

steps to follow:

1. npm install shelljs
2. nexe -i $(pwd)/test.js -o $(pwd)/test
3. ./test

---

results you might see:

module.js:487
    throw err;
    ^

Error: Cannot find module './src/cat'
    at Function.Module._resolveFilename (module.js:485:15)
    at Function.Module._load (module.js:437:25)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at s ([eval]:1:114)
    at [eval]:1:305
    at [eval]:3312:3
    at Array.forEach (native)
    at Object.16../commands ([eval]:3311:23)
    at s ([eval]:1:254)

[nfischer]

@freemine thanks for the update. The steps are clear now. I'll try this out sometime this week and see what I can reproduce.

Please ping this bug if I don't get back to it within a week.

[freemine]

@nfischer just ping in case you're just too busy

[nfischer]

I can reproduce this. It seems like nexe has some serious issues with how our project is laid out.

It can't handle this line because we determine which packages to import at runtime (even though it's from a static list in another JS file).

nexe 2.0.0-rc.4 can handle importing shelljs, but errors if you try to use any methods:

// test.js
const shell = require('shelljs');
shell.echo('good');

$ nexe test.js --build -o test-2
$ ./test-2
/path/to/issue-754/test-2:6
shell.echo('good');
      ^

TypeError: shell.echo is not a function
    at Number.<anonymous> (/path/to/issue-754/test-2:6:7)
    at c (/path/to/issue-754/test-2:4078:3281)
    at Function.r.import (/path/to/issue-754/test-2:4078:3747)
    at d (/path/to/issue-754/test-2:4075:15)
    at Object.<anonymous> (/path/to/issue-754/test-2:4078:1)
    at Module._compile (module.js:569:30)
    at _third_party_main.js:114:20
    at NativeModule.compile (bootstrap_node.js:563:7)
    at Function.NativeModule.require (bootstrap_node.js:506:18)
    at bootstrap_node.js:92:22

I also tried changing the imports to a static list of require()s, but nexe 2.0.0-rc4 gives me:

/path/to/issue-754/test-2:653
  if (shell[name] && !wrapOptions.overWrite) {
           ^

TypeError: Cannot read property 'cat' of null
    at Object._register [as register] (/path/to/issue-754/test-2:653:12)
    at Number.<anonymous> (/path/to/issue-754/test-2:672:8)
    at c (/path/to/issue-754/test-2:6980:3281)
    at w.require (/path/to/issue-754/test-2:6980:3065)
    at Number.<anonymous> (/path/to/issue-754/test-2:40:3)
    at c (/path/to/issue-754/test-2:6980:3281)
    at w.require (/path/to/issue-754/test-2:6980:3065)
    at Number.<anonymous> (/path/to/issue-754/test-2:5:15)
    at c (/path/to/issue-754/test-2:6980:3281)
    at Function.r.import (/path/to/issue-754/test-2:6980:3747)

---

As far as I know, everything we're doing is perfectly legal. @freemine could you CC a dev from nexe so we can discuss next steps?

[nfischer]

CC @calebboyd @jaredallard

Is ShellJS doing something nexe explicitly disallows, or is this an oversight by nexe?

Please add whomever is the best to answer this.

[calebboyd]

@nfischer It looks like the bundler nexe is using is getting hung up on a few of the constructs in the code.

The dynamic requires as you highlighted

shelljs/shell.js

Lines 24 to 26 in c7d65ac

	require('./commands').forEach(function (command) {
	require('./src/' + command);
	});

and

shelljs/src/common.js

Line 8 in c7d65ac

var shell = require('..');

Ideally the bundler would be able to handle that pkg#main field, but it must be a bug. I'll see what can be done to handle that case.
/cc @nchanged

I'll make these issues and see if we can't get it resolved. In the meantime. A harmless fix is, as you mentioned, Declare the commands explicitly, and in common.js change require('..') to require('../shell')`

[nfischer]

Are you parsing require() calls to determine what files to bundle vs. strip from the binary? If so, that's rather error-prone (since dynamic require()s are valid). In fact, we already have .npmignore and the files attribute for that purpose. Npm already uses those to strip files from dependencies, so your work is done for you for dependencies. I could see files and .npmignore being useful for source files in the top-level module though.

I'm not familiar with nexe's architecture, apologies if I'm misinterpreting what you're doing.

[calebboyd]

Yes. The require calls are parsed by the bundler, though nexe's focus is tying in that bundle to a runtime -- Right now we're relying on this bundler to actually assemble the application. It still has some issues I'm trying to identify and work through, hence the rc status...

You're not wrong about pkg#files and .npmignore. Just not all modules include those so its not viable as the primary solution. It could be useful as a backup in cases where they exist and dynamic requires were encountered.

@freemine As an escape, you can pass --bundle ./my-bundle-producer.js to nexe

Here is the signature
Here is an example that works with shell.echo('good')

my-bundle-producer.js

const webpack = require('pify')(require("webpack"))
const fs = require('fs')

module.exports.createBundle = function (options) {
    return webpack({
        entry: options.input,
        target: 'node',
        output: { filename: 'tmp.js' }
    }).then(() => {
        const result = fs.readFileSync('./tmp.js').toString()
        fs.unlinkSync('./tmp.js')
        return result
    })
}

Thanks for finding these issues -- It helps alot!

[nfischer]

As an escape, you can pass --bundle ./my-bundle-producer.js to nexe

Thanks for the workaround! 🎉

Just not all modules include those so its not viable as the primary solution. It could be useful as a backup in cases where they exist and dynamic requires were encountered.

Why not follow npm's behavior? Npm also documents a blacklist of files/directories it never includes; it includes everything else, unless specified otherwise.

Regardless, the bundler definitely should not strip files from dependencies (npm already does all valid stripping). I know some maintainers don't utilize .npmignore, but the right approach is to urge those maintainers to use these tools (then the entire npm ecosystem benefits 😄 ).

It's ok to provide require()-based stripping as an optional optimization, but we can't prohibit legal uses of require() by default.

---

I'm going to close this issue (this is a bundler bug, not a ShellJS bug). Let's continue general discussion below and we can reopen if ShellJS really is doing something invalid.

[calebboyd]

Right. like I mentioned, nexe didn't try to solve/focus on bundling -- Looks like we might need to re-evaluate the solution that was chosen.