ewd-qoper8 icon indicating copy to clipboard operation
ewd-qoper8 copied to clipboard

process.argv[2] !== 'undefined')

Open killmenot opened this issue 7 years ago • 5 comments

Hi @robtweed

Please take a look: https://github.com/robtweed/ewd-qoper8/blob/master/lib/worker/proto/init.js#L36

I believe that line 36 should be

if (process.argv[2] && typeof process.argv[2] !== 'undefined') {

However, I think that

if (typeof process.argv[2] !== 'undefined') {
``
also will work

killmenot avatar Aug 09 '17 15:08 killmenot

In fact I believe my code is correct. If no worker module is defined in the startup callback, then there will be an actual value of 'undefined' in the process argv array. If you look at the various tests, some don't define a module - in which case the default worker handers are used. However test5.js, test6.js etc define a custom module. Try looking at this line in question when you run, eg test4.js versus test5.js and see what ends up in the process.argv[2] array element

robtweed avatar Aug 09 '17 17:08 robtweed

@robtweed

Please take a look at the following screenshot: screenshot 2017-08-10 15 55 47 The content of the test.js file is pretty simple, just console.log(process.argv);

I believe we're talking about line 42 and line 64 here.

If no worker module defined, there is no value for process.argv[2]. In this case the correct condition should be

typeof process.argv[2] !== 'undefined'

because

process.argv[2] && process.argv[2] !== 'undefined'

do a little bit different things:

  1. check if process.argv[2] exists (any non empty value)
  2. check if process.argv[2] is not equal 'undefined' (compare strings)

That's why I believe typeof process.argv[2] !== 'undefined' is more correct rule here

killmenot avatar Aug 10 '17 13:08 killmenot

Other questions:

  1. What if module path contain dot? example: path/to/my.module.file?
  2. Do we need support of relative paths?

At the current moment module can be loaded

  • by absolute path (/var/www/path/to/module)
  • from node_modules folder (/ewd-qoper8/lib/tests/test-workerModule1)

I created examples repository - https://github.com/killmenot/ewd-qoper8-examples/blob/master/examples/test5.js#L38

However, when I run this test file I got the following error:

process.argv[2] = examples/modules/example-worker-module
workerModule: examples/modules/example-worker-module; undefined
module.js:487
    throw err;
    ^

Error: Cannot find module 'examples/modules/example-worker-module'
    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 workerProcess.init (/Users/killmenot/projects/github/killmenot/ewd-qoper8-examples/node_modules/ewd-qoper8/lib/worker/proto/init.js:50:25)
    at Object.<anonymous> (/Users/killmenot/projects/github/killmenot/ewd-qoper8-examples/node_modules/ewd-qoper8-worker.js:5:8)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)

I think if we improve require calls here https://github.com/robtweed/ewd-qoper8/blob/master/lib/worker/proto/init.js#L46-L50 we may allow to load scripts from process.cwd folder at least.

killmenot avatar Aug 10 '17 14:08 killmenot

If you can think of a backward-compatible way to load module other than by absolute path or relative to the node_modules directory, then yes - good idea. But it must be backward-compatible with how it works currently

robtweed avatar Aug 10 '17 16:08 robtweed

Try running the test named benchmark.js, eg:

node benchmark 2 1000 100 100

If you trap the process.argv array in init() you'll see:

process.argv = ["/home/rtweed/.nvm/versions/node/v8.3.0/bin/node","/home/rtweed/qewd/node_modules/ewd-qoper8-worker.js","undefined"]

The code in init() is correct. Change it to what you propose and the benchmark test fails

robtweed avatar Aug 10 '17 16:08 robtweed