search

rotemdan / lzutf8.js

A high-performance Javascript string compression library
MIT License
322 stars 26 forks source link

Fixing commonJS module.export (non-node) #5

Closed TheIronDev closed 9 years ago

TheIronDev commented 9 years ago

I changed the exports logic

from: if(runningInNodeJS()) to: if(module && module.exports)

Otherwise, the assumption is we only want to module.exports in a node environment, which, at least for my webpack scenario doesn't hold :)

Added note: I've been testing this locally with: https://github.com/TheIronDeveloper/jenova-tests/blob/master/public/javascripts/app.js

Using webpack, the first line requires jenova,

On this line, I call a function that ultimately calls lzutf8.compress.

webpack expects module.exports to have taken hold for the browser.

rotemdan commented 9 years ago

Seems reasonable. I guess it could export whenever CommonJS is detected, though it does seems a bit weird to export a CommonJS module that relies on the availability of the DOM, but if it works, I guess it might be OK - as long as it wouldn't break anything.

I committed the change but instead used typeof module === "object" && typeof module.exports === "object" for extra safety to make sure these are actually objects (as weird as it seems, without this check, Javascript would actually allow module to be boolean, and exports to be say, a number, if I'm not mistaken).

In general, I haven't really tested this on special purpose/embedded platforms (Node-Webkit, Cordova, WinJS) and module bundlers. I never used AMD or ES6 modules myself but I guess they could also be supported in the future. Also, you might have noticed the bower.json file in the root directory - it's there but it isn't really registered into the bower repository, for some technical objection I had with it, that I can't remember right now (it's been a while since I last looked at that).

TheIronDev commented 9 years ago

Oh man, once you start using webpack the everything starts to become shiny and exciting! :) JS that works both on client and server the same way is amazing! :)

Anyhoo, Thanks for the fix! I reinstalled lzutf8 and everything started working! :+1:

rotemdan commented 9 years ago

No problem. Please notify me if you have any other issues. I find it both exciting and scary that someone is actually using this library in practice, and even in a cross-platform fashion (I mean considering all the "hacks" and polyfills it applies to run both in the browser and in Node.js, and in IE8/IE9, which don't even support typed arrays).