Globals and exports

[mryellow]

Any reason for deciding to go with globals over checking for browser and deliveringwindow.RL or module.exports.RL?

Something you'd accept as a PR, if it was done in a way that fits?

[karpathy]

Hi, there is no reason for this except that I don't know how it should be done properly. If you can fix this with a PR I'd be happy to merge

[karpathy]

is this a big change? I'm not exactly sure what's involved. And is this done so that things work transparently on both node and browser? Or what's wrong with the current way? It does create two new globals R and RL that hold the lib.

[mryellow]

how it should be done properly

Looks like that check pattern you included on the ConvnetJS which dumps it into window.foo or module.exports.foo is best practice. Not something I've done a million times either....

is this done so that things work transparently on both node and browser?

I believe that check takes care of all the cases. Anything which would require global instead of window?

Now what is wrong with the current way..... Maybe nothing....

You're keeping everything simple and flat without package managers, maybe the best bet is a separate wrapper which can be used with node, which simply does the checks and throws everything in exports regardless.

[mryellow]

I've wrapped it using this pattern, once Math.tanh is polyfilled for NodeJS v0.10.40 everything works as expected.

execfile.js

var vm = require("vm");
var fs = require("fs");
module.exports = function(path, context) {
  context = context || {};
  var data = fs.readFileSync(path);
  vm.runInNewContext(data, context, path);
  return context;
};

foo.js

var execfile = require("./execfile");

// Polyfill ES6.
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/tanh
Math.tanh = Math.tanh || function(x) {
  if (x === Infinity) {
    return 1;
  } else if (x === -Infinity) {
    return -1;
  } else {
    var y = Math.exp(2 * x);
    return (y - 1) / (y + 1);
  }
};

var RL = execfile('./lib/rl.js', {
  Math: Math // Not certain this was needed...
}).RL;

[Rob-Voss]

Would something like this be an option? https://alicoding.com/write-javascript-modules-that-works-both-in-nodejs-and-browser-with-requirejs/

[mryellow]

Yeah similar to this:

https://github.com/karpathy/convnetjs/blob/master/src/convnet_export.js

Think in the end it's probably not really needed, depends on if this lib is about being a block of code in one file, simple and with much of the bones showing... or a package for people to install and run with.