aheckmann / gm

GraphicsMagick for node
http://aheckmann.github.com/gm/
6.95k stars 616 forks source link

browserify gm #474

Closed mt-sergio closed 7 years ago

mt-sergio commented 8 years ago

I'm trying to execute a lambda module which uses gm, but this line makes it crash when I try to browserify:

module.exports.version = JSON.parse(
  require('fs').readFileSync(__dirname + '/package.json', 'utf8')
).version;

Any idea? I will solve it just excluding the module, but I would prefer to do it better.

PD: I'm using jaws framework

positlabs commented 8 years ago

I'm not familiar with jaws, but are you able to install the graphicsmagick CLI binaries in a lambda function? I assume it would need the binary download to be managed via npm install, which this lib currently doesn't do.

mt-sergio commented 8 years ago

Yes, aws lambda uses an AMI which includes gm by default. jaws became serverless and maybe now the newest version of browserify handles this correctly...

positlabs commented 8 years ago

Are you seeing an error when it crashes?

Why are you using browserify? This code isn't meant to run in a browser, is it?

mt-sergio commented 8 years ago

browserify is a library to compress and minimize the output code (everything in 1 file). It's widely used for increase the lambda performance.

positlabs commented 8 years ago

I really doubt that browserify will manage references to __dirname automatically. If you flatten all of your required modules into a single file, __dirname might not be what it used to be (which will cause the fs.readFileSync call to fail).

Actually, I just tested this and for some reason, browserify intentionally makes __dirname undefined. Dunno why, but it does. There's one reason for failing.

Another reason it fails is because browserify doesn't resolve built-in node modules like fs. It kind of does the opposite in that it defines it as an empty module with nothing to export. Again, I don't know exactly why this happens, but it does.

Even if fs does exist in the environment you run the compiled code in, it will fail because the browserified require() method isn't the same as the native node require(). One solution might be to point the native require statements to some global like global.originalRequire = require, then use that to pull in built-in node modules.

Anyway, I still don't think browserify is quite what you are looking for in terms of compiling node modules (especially if they use built-in node stuff). There are a few libs out there for packing up modules, but I haven't tried them.

https://www.npmjs.com/package/nexe http://enclosejs.com/

mt-sergio commented 8 years ago

browserify doesn't handle dynamic requires, that's why it fails. I was asking to change the code of this module to point statically to the file, but for me is fine, I just patched it myself.

positlabs commented 8 years ago

Ah, ok. I thought you were asking why it was failing. You could submit a PR of your patch, yeah?

mt-sergio commented 8 years ago

My solution was to exclude this module, so I cannot submit any patch. I think someone with knowledge about gm should fix all the code as there are more of these compatibilities issues.

positlabs commented 8 years ago

I still think browserify is the wrong tool for what you are using it for.

Browserify lets you require('modules') in the browser...

I'm not sure it's worth supporting since this lib is most definitely meant to run on a server, and there are other perfectly viable methods for bundling modules.

Of course, this is just my opinion. Maybe someone else can weigh in? Ultimately it sounds like someone familiar with the code would need to be willing to fix it, so they would be the deciding factor.

mt-sergio commented 8 years ago

https://github.com/serverless/serverless-optimizer-plugin

positlabs commented 8 years ago

Perfect!