Fix in CommonJS support #285

Closed
wants to merge 1 commit into from

2 participants

@AMorgaut

The current CommonJS support is node.js specific as module.exports is
not part of CommonJS and is then not supported in all implementations
(ex: Rhino, Wakanda, CouchDB, ArangoDB, TeaJS...).
This correction make Mustache fully CommonJS compliant while being
still compatible with AMD, the script tag, the web worker
importScripts, and of course node.js

Alexandre Morgaut Fix the CommonJS support
The current CommonJS support is node.js specific as module.exports is
not part of CommonJS and is then not supported in all implementations
(ex: Rhino, Wakanda, CouchDB, ArangoDB, TeaJS...).
This correction make Mustache fully CommonJS compliant while being
still compatible with AMD, the script tag, the web worker
importScripts, and of course node.js
7a240f5
@AMorgaut

I realized that my pull request introduces the dependency with the ES5 Object.create(), and more specifically the property descriptor setter which can not be polyfilled from ES3.

A "not that pretty", but very simple, alternative would be to replace in the current implementation

    module.exports = factory; // CommonJS

by

    for (var api in factory) {
        exports[api] = factory[api]; // CommonJS
    }

Notes:

  • the "for in" need unfortunately to be used instead of getOwnPropertyNames() to be ES3 compliant
  • the context is changed from the internal exports to the external one, but the closure variables remain shared between the api methods
@mjackson
Collaborator

What do you think about fixing this issue like this? It eliminates the use of module.exports by simply reusing the existing exports object as the argument to the factory function, and is a bit more elegant than using a for..in.

@mjackson mjackson added a commit that closed this pull request Apr 23, 2013
@mjackson mjackson Don't use module object in CommonJS
Fixes #308
Fixes #285
f8bfbd2
@mjackson mjackson closed this in f8bfbd2 Apr 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment