Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-26403] kitchensink-v2 broken on latest SDK build on master

GitHub Issuen/a
TypeBug
PriorityHigh
StatusClosed
ResolutionFixed
Resolution Date2018-10-22T15:06:32.000+0000
Affected Version/sRelease 7.5.0
Fix Version/sRelease 7.5.0, node-titanium-sdk 0.6.4
ComponentsAndroid, CLI, iOS, Windows
Labelsn/a
ReporterSamir Mohammed
AssigneeChristopher Williams
Created2018-09-20T13:26:14.000+0000
Updated2018-11-22T16:37:25.000+0000

Description

On the latest version of the SDK in master (7.5.0.v20180920040518) kitchensink-v2 is broken and displays the following error on iOS (Looks like it broke in SDK Version 7.5.0.v20180919143608): 
[ERROR] :  Script Error {
[ERROR] :      column = 18;
[ERROR] :      line = 6;
[ERROR] :      message = "Can't find variable: _";
[ERROR] :      sourceURL = "file:///Users/Samir/Library/Developer/CoreSimulator/Devices/2BA65AF9-7B80-486E-9A20-005A318A4424/data/Containers/Bundle/Application/5A64DDEB-EA50-4721-81B9-15B9BBC5D6DE/KitchenSink.app/logger.js";
[ERROR] :      stack = "    at Logger(/logger.js:6:18)\n    at (/logger.js:33:41)\n    at global code(/logger.js:35:70)\n    at require@[native code]\n    at (/alloy/controllers/index.js:5:22)\n    at global code(/alloy/controllers/index.js:108:70)\n    at require@[native code]\n    at createController(/alloy.js:339:21)\n    at (/app.js:34:23)\n    at global code(/app.js:36:70)\n    at require@[native code]\n    at (/ti.main.js:27:10)\n    at loadAsync(/ti.internal/bootstrap.loader.js:106:11)\n    at global code(/ti.main.js:24:52)";
[ERROR] :      toJSON = "<KrollCallback: 0x604000265e00>";
[ERROR] :  }
[ERROR] :  Script Error Module "logger.js" failed to leave a valid exports object
[ERROR] :  Script Error Module "alloy/controllers/index.js" failed to leave a valid exports object
[ERROR] :  Script Error Module "app.js" failed to leave a valid exports object
*Android Error* 
[ERROR] :  TiExceptionHandler: (main) [213,213] /logger.js:6
[ERROR] :  TiExceptionHandler: 		this.logger = _.extend({}, Backbone.Events);
[ERROR] :  TiExceptionHandler:                 ^
[ERROR] :  TiExceptionHandler: ReferenceError: _ is not defined
[ERROR] :  TiExceptionHandler:     at new Logger (/logger.js:6:17)
[ERROR] :  TiExceptionHandler:     at /logger.js:33:33
[ERROR] :  TiExceptionHandler:     at Module._runScript (ti:/module.js:613:9)
[ERROR] :  TiExceptionHandler:     at Module.load (ti:/module.js:105:7)
[ERROR] :  TiExceptionHandler:     at Module.loadJavascriptText (ti:/module.js:457:9)
[ERROR] :  TiExceptionHandler:     at Module.loadAsFile (ti:/module.js:512:15)
[ERROR] :  TiExceptionHandler:     at Module.loadAsFileOrDirectory (ti:/module.js:429:20)
[ERROR] :  TiExceptionHandler:     at Module.require (ti:/module.js:296:17)
[ERROR] :  TiExceptionHandler:     at require (ti:/module.js:570:15)
[ERROR] :  TiExceptionHandler:     at /alloy/controllers/index.js:5:15
[ERROR] :  TiExceptionHandler:
[ERROR] :  TiExceptionHandler:     org.appcelerator.kroll.runtime.v8.V8Runtime.nativeRunModule(Native Method)
[ERROR] :  TiExceptionHandler:     org.appcelerator.kroll.runtime.v8.V8Runtime.doRunModule(V8Runtime.java:180)
[ERROR] :  TiExceptionHandler:     org.appcelerator.kroll.KrollRuntime.runModule(KrollRuntime.java:247)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiLaunchActivity.loadActivityScript(TiLaunchActivity.java:135)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiLaunchActivity.windowCreated(TiLaunchActivity.java:190)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiRootActivity.windowCreated(TiRootActivity.java:174)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiBaseActivity.onCreate(TiBaseActivity.java:675)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiLaunchActivity.onCreate(TiLaunchActivity.java:176)
[ERROR] :  TiExceptionHandler:     org.appcelerator.titanium.TiRootActivity.onCreate(TiRootActivity.java:163)
[ERROR] :  TiExceptionHandler:     android.app.Activity.performCreate(Activity.java:6237)
[DEBUG] :  OpenGLRenderer: Use EGL_SWAP_BEHAVIOR_PRESERVED: true
[ERROR] :  GraphResponse: {HttpStatus: 400, errorCode: -1, subErrorCode: -1, errorType: null, errorMessage: null}
[DEBUG] :  D/com.facebook.FacebookSdk: getGraphApiVersion: v3.0
[ERROR] :  V8Exception: Exception occurred at /logger.js:6: Uncaught ReferenceError: _ is not defined
*Test Steps*

Download the latest SDK version from master appc ti sdk install -b master -d

Download the kitchensink-v2 app from https://github.com/appcelerator/kitchensink-v2

Run the kitchensink-v2 using the latest master build

*Expected result* Application should run without any issues *Actual result* Above error is shown *Note:* kitchensink-v2 works as expected in SDK version 7.5.0.v20180919120117

Comments

  1. Ewan Harris 2018-09-20

    This might be down to a change in the handling of the global scope-ness of variables defined in app.js, Alloy uses this to expose Alloy, _, and Backbone variables to the global scope for a user.
  2. Joshua Quick 2018-09-20

    Oh bummer. This isn't just an Alloy issue. I can reproduce this issue in a Classic app as well. How it's supposed to work is that all local var declared variables in the 1st script executed (in our case "app.js") must have global scope across all JS files in the app. JavaScript does this for us by default. What's changed is that we're launching with "ti.main.js" instead of "app.js" now. I wasn't aware of this behavior. I'm kind of bummed by this. :-/ *Steps to reproduce:*

    Set up an "app.js" with the below.

    Build and run using Titanium 7.3.0.

    On app startup, an alert will read: "global.test: Hello World"

    Build and run using Titanium 7.5.0.

    On app startup, an alert will read: "global.test: undefined" _(This is the bug.)_

       var test = "Hello World";
   alert("global.test: " + global.test);
    _*Edit:*_ _If you build and run the above code with 7.5.0 and LiveView, then it works. LiveView appears to have a work-around for this issue since it's effectively a bootstrap script as well._
  3. Ewan Harris 2018-09-20

    For the liveview part, I imagine that's to do with [this piece of code](https://github.com/appcelerator/liveview/blob/master/lib/fserver.js#L298-L317) which removes var/let/const from all variable declarations in app.js when serving the code up to the device causing them to become globals variables
  4. Joshua Quick 2018-09-21

    FYI: [~kiguchi] and I are discussing this on the following Windows PR... https://github.com/appcelerator/titanium_mobile_windows/pull/1285#issuecomment-423415599 We "can" solve this by using the eval() function (which I think is a naughty solution), but we would have to ensure that stack-traces and debugger breakpoints work.
  5. Christopher Williams 2018-09-24

    FWIW, Alloy is really relying on undefined/undocumented/unspec'd behavior. Ideally all of our code should assume we're going to be treated as a "module" (CommonJS or ES6) and therefore shouldn't implicitly assume that top-level vars will live on as globals so long as they're defined in the eventual app.js. I think we should fix Alloy immediately to future-proof it here: we have explicitly exposed a global object to use for hanging global functions/variables/state. (Also a note that we had undocumented behavior that the this binding referred to the global object in app.js before as well)
  6. Christopher Williams 2018-09-24

    Oh yeah - and we had written a custom babel plugin to handle references to this in the top-level so that when we transpiled we'd replace with a reference to global. I assume we should be able to do the same exact thing here - gather all top-level variables and assign them as properties off global. It'd preserve backwards compatibility so long as the plugin got run. (See https://github.com/appcelerator/node-titanium-sdk/blob/master/lib/global-this.js)
  7. Ewan Harris 2018-09-24

    https://github.com/ewanharris/liveview/blob/TIMOB-25861/lib/global-plugin.js Might fit the bill for the global plugin, I wrote it for liveview transpilation support as it needs to do the same task as we need to achieve here (not sure why I didn't think of linking that in before), we'd just need to separate the transpile flag from running babel I assume, and remove the lvGlobal bits from it
  8. Joshua Quick 2018-09-24

    If our best practices state that you should not depend on "app.js" globals, then how about we add the following to the top of the [./app/lib/logger.js](https://github.com/appcelerator/kitchensink-v2/blob/master/app/lib/logger.js) file. The below will solve the problem for kitchensink-v2. (Note that Alloy injects the below code at the top of every controller JS file.) 
       var Alloy = require('/alloy'),
   	Backbone = Alloy.Backbone,
   	_ = Alloy._;
    We can then write-up a separate ticket for the bootstrap breaking-change. Sounds good?
  9. Ewan Harris 2018-09-26

    My opinion is that even if we change the app.js global scope behaviour, we should still make Alloy, Backbone and _ globals ourselves in Alloy, this is the route that Fokke took in his PR for ALOY-1259 (a kind of related ticket) [here](https://github.com/appcelerator/alloy/pull/675). Requiring a developer to repeat the same 3 lines in most controller files seems meh to me, and there may be internal Alloy files that rely on that behaviour, not making those globals anymore just seems like a needless breaking change imo. Could you clarify on bootstrap breaking-change do you mean the change that caused this ticket? I'm not 100% certain why we'd break off from this one?
  10. Christopher Williams 2018-09-26

    My two cents: - File a new ticket to fix alloy to properly assign the globals. - Add a custom babel plugin that fixes the issue on app.js top-level vars by assigning them to global as properties explicitly. Possibly bake in some logging there to note that this is deprecated behavior we will eventually remove. - Always run this babel plugin, even if transpilation is turned off? I know that tehcnically speaking there are ways we could retain this behavior. On Android we have internal APIs for running JS code in a specific context. On Windows/iOS JSC allows us to pass in the this binding when evaluating code, etc. We could use eval to work around it. But I think all of these solutions aren't ideal, as we'd definitely need to tweak Windows/iOS to expose them, and it just doesn't feel very elegant to me. Given that people are really relying on unspecified/documented behavior and that this is known to be a bad practice we should simply go with the easier fix of using the babel plugin to implicitly fix it for them (while complaining in the logs about usage), and deprecate it so that we can remove it in 9.0.0.
  11. Ewan Harris 2018-10-01

    I've taken the liveview plugin I wrote and converted it for general usage, the PR is here https://github.com/appcelerator/node-titanium-sdk/pull/47. It just needs a little bit more work to try figure out if it's possible to pass our logger instance in there
  12. Christopher Williams 2018-10-02

    There's a few fixes here to tackle this. Specifically, we should update Alloy to explicitly expose any globals when there's a global object available. Second, Ewan is working on a babel plugin to fix the general case of implicit globals from top-level variables in app.js.
  13. Christopher Williams 2018-10-02

    Here's the alloy fix: https://github.com/appcelerator/alloy/pull/913
  14. Ewan Harris 2018-10-03

    titanium_mobile: https://github.com/appcelerator/titanium_mobile/pull/10364 titanium_mobile_windows: https://github.com/appcelerator/titanium_mobile_windows/pull/1288
  15. Christopher Williams 2018-10-22

    All PRs have been merged for this ticket.
  16. Keerthi Mahalingam 2018-10-22

    Verified the fix on SDK 7.5.0.v20181018133006. Kitchensink-v2 app works fine on android and iOS. Closing. 
        Operating System
      Name                        = Mac OS X
      Version                     = 10.13.6
      Architecture                = 64bit
    Node.js
       Node.js Version             = 8.12.0
      npm Version                 = 6.4.1
    Titanium CLI
      CLI Version                 = 5.1.1
    Titanium SDK                = 7.5.0.v20181018133006
    Device                         =Samsung s5 android 6 device, pixel  android 7 emulator
                                      iPhone 6s iOS 12 device, iPhone 6 iOS 11 simulator

JSON Source