Fix invalid variable name `key` in commonjs environnement.

[nopnop]

Hello,

Look like a typo error : the exported value by keymaster in a commonjs environnement is a variable that do not longer exists : key : changed to assignKey.

[madrobby]

I'm not quite sure what this does, why it is a problem, and how to test it. Can you explain in some more detail?

[nopnop]

Sure,

The line below is used to exports a value in a CommonJS or an AMD environnement:

if(typeof module !== 'undefined') module.exports = key;  

But the variable key is not declared in this scope (no var key = ... or function key() ...).

Actually, this will work most of the time because you assume that global is really the global scope of the runtime:

• This is true when you use keymaster.js without any wrapper (global === window)
• This is probably true with fake commonjs wrapper for browser like ender.js ?
• This may be true most of the time with AMD wrapper like RequireJS depending on the way you use it.

But this is not true when you are in a real CommonJS environnement because your global variable is not the real global variable when you do this:

!(function(global) { 
    // ...
})(this);

In this code, this is === module, not the CommonJS global scope:

// The real global variable:
var realGlobal = global;

!(function(global) { 
    console.log(global === realGlobal); // FALSE
    console.log(global === module); // TRUE

    global.key = 'foobar';

    module.exports = key; // Throw "Uncaught ReferenceError: key is not defined"
})(this);

Anyway. You may think that keymaster.js has nothing to do with CommonJS environnement (usually used server side, with node.js for instance). But, the fact is: there is many project that offer true-CommonJS wrapper to generate browser-side code (like Browserify). This is very useful to bundle, optimize, test, share and reuse some code between the browser side and the server side in very large project.

This line is the only one required to make keymaster.js compatible with a CommonJS environment:

if(typeof module !== 'undefined') module.exports = key; 

The problem with the code above is not specific to any underground third-party wrapper: this is just a variable scope mix-up. It is cleaner (and in this case, required) to exports something you really have in your scope without assuming that keyis in the global scope. Something you really have under control:

// Could be `global.key` because global is declared in the main function scope
// as a parameter:
if(typeof module !== 'undefined') module.exports = global.key; 

// Or could be the `assignKey` function, declared in the main function scope
// because `assignKey` is what you really what to expose when you
// do `global.key = assignKey` 
if(typeof module !== 'undefined') module.exports = assignKey; 

You may test all of this with any CommonJS wrapper. Browserify is the most popular (nb: it produce a lot of boilerplate code for small project):

# In a shell, after browserify installation:
browserify keymaster.js > keymaster.commonjs.js
# Then use keymaster.commonjs.js in the browser.

I hope I have been able to explain my pull request motivations (and please sorry if not: english is not my native language). And many thanks for this great library !

[QuentinFchx]

@madrobby this is definitely an issue we ran into using browserify. I'm waiting for this to be merged and pushed :) Thanks !