circosJS icon indicating copy to clipboard operation
circosJS copied to clipboard

Further information needed in the README Installation section

Open iamtmrobinson opened this issue 6 years ago • 2 comments

The installation section currently says to use npm install --save circos with no further instruction.

Doing just this and then using Browserify to require circos results in the following error: Error: Cannot find module '!!../../node_modules/css-loader/index.js!./tooltip.css' from '/.../node_modules/circos/dist'

Is there an assumption that Webpack is being used by those who try to import circos from npm? Regardless, it would be useful for the documentation to be fleshed out to cover the different ways of running the Circos package.

iamtmrobinson avatar May 30 '18 14:05 iamtmrobinson

I've just hit the same problem when trying to use circos with budo/browserify...

vorg avatar Feb 15 '19 07:02 vorg

This issue really needs to be addressed since circos.js (as of current version 2.1.0) is unusable with browserify.

The Issue

As @iamtmrobinson points out circos.js does assume that it's being used under webpack. But the place where this assumption is breaking browserify is this line in circos.es6.js with a webpack-specific require.

It is very clear that this line is there for hot module replacement for development purposes and is hidden from execution by being inside an if (false) block which I assume circos.js devs turn to true in their dev environment. This is problematic with browserify because (from the website):

Browserify parses the AST for require() calls to traverse the entire dependency graph of your project.

which means it will see any require anywhere in code even if it's unreachable.

Solution

I'm not too familiar with webpack but I think the cleanest solution is for Circos to use __webpack_require__ or some other function for this webpack-specific line. Even doing something like this will work too:

var someOtherNameForRequire = require;
someOtherNameForRequire('!!../../node_modules/css-loader/index.js!./tooltip.css');

While that hasn't happened, people who need to make circos.js work with browserify need to patch the official release. It would be nice if there was a browserify ignore tag that we could add above that line which does not exist (see discussion of this here). For the time being we can just comment out the entire if (false) block. Here is my patch, applicable via patch -p1 < circos.patch:

diff --git a/node_modules/circos/dist/circos.es6.js b/node_modules/circos/dist/circos.es6.patched.js
index 67460f6..7e7e7b1 100644
--- a/node_modules/circos/dist/circos.es6.js
+++ b/node_modules/circos/dist/circos.es6.patched.js
@@ -18454,6 +18454,10 @@ if(typeof content === 'string') content = [[module.i, content, '']];
 var update = __webpack_require__(457)(content, {});
 if(content.locals) module.exports = content.locals;
 // Hot Module Replacement
+/*
+ * This block is unreachable and used for development purposes only; comment it
+ * out so the `require` invocation does not confuse browserify
+ */ /*
 if(false) {
        // When the styles change, update the <style> tags
        if(!content.locals) {
@@ -18466,6 +18470,7 @@ if(false) {
        // When the module is disposed, remove the <style> tags
        module.hot.dispose(function() { update(); });
 }
+*/

 /***/ }),
 /* 455 */

amirkdv avatar Apr 14 '19 16:04 amirkdv