Generate bundle.min.cjs in Node.js install script

[mjethani]

Following up from the discussion here: ghostery/adblocker#2091 (comment)

The Node.js install script now generates a bundle.min.cjs file.

[mjethani]

Note that these dependencies are a bad idea. Ideally the benchmark package should do all of this on its own. But in the interest of moving things forward it's OK for now.

[gorhill]

I was happy to not have external dependencies... Maybe eventually this could be a special make target which would patch package.json to add those dependencies -- not sure if this makes sense

[mjethani]

I don't think this bundle should be provided at all in the end. I'll try to do this in the benchmark repo and if it works out alright then this stuff can be removed.

[gorhill]

I was happy to not have external dependencies... Maybe eventually this could be a special make target which would patch package.json to add those dependencies -- not sure if this makes sense

[mjethani]

Follow-up here: ghostery/adblocker#2095

[gorhill]

I get an error when I do node install:

Error: Cannot find module 'minimist'
Require stack:
- /usr/share/nodejs/rollup/bin/src/index.js
...

---

Never mind, I realize I have to make install-nodejs.

[mjethani]

@gorhill sorry I used the wrong indentation level here.

I notice a few jshint comments in the source. Are you using this tool? Maybe you should add the configuration to the repo along with a lint target in Makefile.

[gorhill]

.jshintrc is already present in the repo.

As for the lint command, I am going to have to read to find out best way to do this -- here is says to add

lint:
    ./node_modules/.bin/jshint ...

Which means I need to add a ./node_modules/.bin/jshint target as well I guess? My installation of jshint is outside the ublock project and I use a specific path to access the command in my text editor, not suitable for the public Makefile here.

[mjethani]

.jshintrc is already present in the repo.

Sorry, my bad!

Which means I need to add a ./node_modules/.bin/jshint target as well I guess? My installation of jshint is outside the ublock project and I use a specific path to access the command in my text editor, not suitable for the public Makefile here.

It's OK to expect people to install jshint manually via npm install -g jshint. Maybe this should be added to the CONTRIBUTING.md file.

As for the lint target, let me see how this works on the command line since I haven't used it.

[mjethani]

It seems JSHint removed indentation level checks a long time ago: https://github.com/jshint/jshint/releases/tag/2.5.0

I'll see if I can add some basic ESLint configuration that matches the JSHint configuration and also adds options for indentation, etc. to match the current coding style.