Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-5406] Android: commonjs exports object is undefined in some cases

GitHub Issuen/a
TypeBug
PriorityHigh
StatusClosed
ResolutionFixed
Resolution Date2011-11-21T08:05:15.000+0000
Affected Version/sRelease 1.8.0
Fix Version/sSprint 2011-46, Release 1.8.0.1
ComponentsAndroid
Labelsparity
ReporterRick Blalock
AssigneeBill Dawson
Created2011-10-03T14:13:00.000+0000
Updated2014-06-19T12:46:31.000+0000

Description

Sample code: 
// app.js
var testmodule1 = require('testmodule');
Ti.API.info('TEST MODULE 1 TEST: ' + testmodule1.someprop); // This will be undefined

// Module
// This doesn't work
exports = {
   someprop: 'coolbeans'
};

Comments

  1. Bill Dawson 2011-10-05

  2. Kevin Whinnery 2011-10-18

    I'll grant that mine was a liberal reading of the CommonJS spec, but they shouldn't have called exports a free variable in the spec if it actually wasn't one (I think I am in the right on the Math/CS definition of free variable). In practice module.exports = {}; is non-standard, but it serves to address the lack of direct assignment to exports, which is either a flaw in either the spec or in many implementations of it, depending on your opinion. So, do we suffer a flaw in the spec because that's what we think the spec dictates? Or do we provide a workable solution for users? I vote for the latter. This: 
       function MyObject() {}
   exports.MyObject = MyObject;
   
       var MyObject = require('MyObject').MyObject;
    is a clumsy implementation. I don't think module.exports is the right answer, though, because it introduces another magical variable that has no place in the spec. A common sense approach would be to allow direct assignment to the exports object. 
       function MyObject() {}
   exports = MyObject;
   
       var MyObject = require('MyObject');
    To view the issue from a parity standpoint, do we want to take away a useful feature from one platform because we don't think it's standard? If it's not a maintainable solution on the Android side then maybe we do, but that's too bad. Sidenote: @Bill your mother is considered homely by many.
  3. Bill Dawson 2011-10-18

    I know, she totally fooled Dad somehow. :) We're transitioning into using node.js's commonjs implementation (via the V8 team -- though it will be available for both V8 and Rhino AFAIK). So I imagine module.exports will automatically become available, since that's a node.js thang. So we can wait and see on that. Meanwhile this can just stay open till we can test the newer implementation.
  4. Bill Dawson 2011-11-21

    Since we're now using node.js's module implementation, we support module.exports = { ... };, which is functionally equivalent to what is desired here.
  5. Bill Dawson 2011-11-21

    So the test case would be: 
       // app.js
   var testmodule1 = require('testmodule');
   Ti.API.info('TEST MODULE 1 TEST: ' + testmodule1.someprop); // This should show 'coolbeans'
   
       module.exports = {
      someprop: 'coolbeans'
   };
  6. Ivan Skugor 2011-11-28

    Hi. :) Just to add a bit more explanation to this discussion. I would like to explain why object can't be assigned directly to the "export" variable. The problem lies in some JS environments that can implement CommonJS (browser to be precise). In browser, scripts loaded via dynamically created script tag are evaluated in global scope. Because of that, module scope has to be improvised by creating new function scope that wraps module's source code and in which "exports" and "require" variables are injected via function parameters. Basically, after wrapping of module's source code this is created: 
       function(require, exports) {
       //loaded module's source code
   }
    where "require" is a function (that loads dependencies) and "exports" is basically empty object - {} to which API is added. Changes to that object needs to be reflected on external object injected to that function because in module's wrapper function there is no return statement. And here the problem with assigning object (or anything else) directly to "exports" variable lies. If anything would be directly assigned to the "exports" variable, that change would not be reflected on the external object (because object are passed by the copy of reference, so "exports" variable contains reference copy of external object and changing what that variable points at, does not change that external object or other references to that external object). "module.exports" is a workaround actually. That said, I think you should be careful with CommonJS implementation in Mobile SDK's if you want Ti-related source code to work in browsers (or Mobile Web SDK's) - code portability is basic idea of CommonJS. Also, my bug report regarding global variables in CommonJS also suffers from similar problem. Anyway ... as you may noticed, CommonJS is not perfect, but new ECMAScript spec will have module system (which I hope will fix some of CommonJS issues) so maybe it should be taken into consideration right now (I don't know the current state of module specification, but I know there was some work involved).
  7. Dustin Hyde 2011-12-21

    Verified as Fixed. SDK: 1.8.0.1.v20111220190134 Android Runtimes: V8/Rhino Studio: 1.0.7.201112152014 OS: OS X Lion Devices Tested: Android Emulator 2.2, iPhone Simulator 5.0
  8. Lance Spellman 2011-12-25

    Confirmed, ran test above on a 5.0 simulator with same SDK and Studio versions. Worked using: //test.js module.exports={someprop:'testing'}; //app.js var test1 = require('test'); alert(test1.someprop); // outputs testing Also confirmed this generates script { error on 1.7.5 SDK

JSON Source