TIMOB-9874: BlackBerry: implement require() #96

dlifshitz-maca · 2012-07-09T17:24:40Z

Reviewers: Alex, Harut

[Issues Fixed]
TIMOB-9874: BlackBerry: implement require()

[Changes]
NativeLoggerWorker.cpp
- add newline to message first so the stream doesn't split it up

TiRootObject.cpp
- function executeScript
  - use variables for the bootstrap and app filenames
  - pass the filenames to the calls to Compile()
  - add the javascript filename and line number to the log message
- implement function _require

TiTitaniumObject.cpp
- function _include
  - add the javascript filename and line number to the log message

[Tests]
Test 1: Build and run an app (using .js files from Jira)
1) Put the .js files in the Resources dir so they will get packaged
2) Build, package, and run the app
3) Verify the correct info is shown in the app
4) Close the app, get the log, and verify the "module being loaded!" message is only shown once

Test 2: Crash the app
1) Put an undeclared variable in app.js, a file being included, and a file being required
2) Run the app, verify it crashes, get the log, and verify the location of the problem is shown
3) Remove the offending variable
4) Repeat Steps 2-3 until all offending variables are gone and the app runs successfully

[Issues Fixed] TIMOB-9874: BlackBerry: implement require() [Changes] NativeLoggerWorker.cpp - add newline to message first so the stream doesn't split it up TiRootObject.cpp - function executeScript - use variables for the bootstrap and app filenames - pass the filenames to the calls to Compile() - add the javascript filename and line number to the log message - implement function _require TiTitaniumObject.cpp - function _include - add the javascript filename and line number to the log message [Tests] Test 1: Build and run an app (using .js files from Jira) 1) Put the .js files in the Resources dir so they will get packaged 2) Build, package, and run the app 3) Verify the correct info is shown in the app 4) Close the app, get the log, and verify the "module being loaded!" message is only shown once Test 2: Crash the app 1) Put an undeclared variable in app.js, a file being included, and a file being required 2) Run the app, verify it crashes, get the log, and verify the location of the problem is shown 3) Remove the offending variable 4) Repeat Steps 2-3 until all offending variables are gone and the app runs successfully

alexandergalstyan · 2012-07-10T07:52:32Z

blackberry/tibb/TiRootObject.cpp

-        String::Utf8Value error(tryCatch.Exception());
-        TiLogger::getInstance().log(*error);
+        Local<Value> exception = tryCatch.Exception();
+        if (exception->IsString())


Is that something, that could happen? I don't think it could be string.

I put a FIXME to handle this better

alexandergalstyan · 2012-07-10T08:41:19Z

Are we able to load module with its ID? If not please add todo for that.

alexandergalstyan · 2012-07-10T08:41:40Z

Update apidoc, since it says NYI for BlackBerry

alexandergalstyan · 2012-07-10T08:41:45Z

Reviewed

dlifshitz-maca · 2012-07-10T18:55:01Z

For the ID, isn't that just the filename without the ".js"? If not then that would be something we'd need to investigate more.

updated require() documentation put module error string in central location check if Message() is empty replace exception->IsString() hack with FIXME rename v8Id to be more representative add space in include() error string

dlifshitz-maca · 2012-07-10T18:58:51Z

Updated apidoc

dlifshitz-maca · 2012-07-10T18:58:58Z

Updated patch

alexandergalstyan · 2012-07-11T07:06:51Z

For the ID I meant the native modules referenced with say com.macadamian.module1. Please add todo, so we can track it later.

alexandergalstyan · 2012-07-11T07:07:09Z

Approved with comment

jpl-mac · 2012-07-11T21:53:22Z

apidoc/Global/Global.yml

@@ -180,7 +180,8 @@ methods:
                require ('/myModule/module.js')

        #### BlackBerry
-            NYI
+
+            Currently, only base paths are valid. Relative paths are NYI.


Have you created a JIRA task to track that NYI stuff? I can't see it from the commit description.

Harutyun · 2012-07-12T12:26:06Z

Approved with comment

added id support to TODO

Conflicts: blackberry/tibb/TiMessageStrings.h

dlifshitz-maca · 2012-07-12T18:30:28Z

@alexandergalstyan I've added a TODO
@Harutyun, @jpl-mac I'll be creating jiras

TIMOB-9874: BlackBerry: implement require() [Issues Fixed] TIMOB-9874: BlackBerry: implement require() [Changes] NativeLoggerWorker.cpp - add newline to message first so the stream doesn't split it up TiRootObject.cpp - function executeScript - use variables for the bootstrap and app filenames - pass the filenames to the calls to Compile() - add the javascript filename and line number to the log message - implement function _require TiTitaniumObject.cpp - function _include - add the javascript filename and line number to the log message [Tests] Test 1: Build and run an app (using .js files from Jira) 1) Put the .js files in the Resources dir so they will get packaged 2) Build, package, and run the app 3) Verify the correct info is shown in the app 4) Close the app, get the log, and verify the "module being loaded!" message is only shown once Test 2: Crash the app 1) Put an undeclared variable in app.js, a file being included, and a file being required 2) Run the app, verify it crashes, get the log, and verify the location of the problem is shown 3) Remove the offending variable 4) Repeat Steps 2-3 until all offending variables are gone and the app runs successfully

dlifshitz-maca · 2012-07-12T19:27:56Z

Created Jiras:
https://jira.appcelerator.org/browse/TIMOB-9950 BlackBerry: Fix/implement relative include/require
https://jira.appcelerator.org/browse/TIMOB-9953 BlackBerry: Fix logging/exception tags and exception output

alexandergalstyan reviewed Jul 10, 2012
View reviewed changes

TIMOB-9874: BlackBerry: implement require()

1411848

updated require() documentation put module error string in central location check if Message() is empty replace exception->IsString() hack with FIXME rename v8Id to be more representative add space in include() error string

jpl-mac reviewed Jul 11, 2012
View reviewed changes

dlifshitz-maca added 2 commits July 12, 2012 14:03

TIMOB-9874: BlackBerry: implement require()

a51e743

added id support to TODO

Merge branch 'master' into TIMOB-9874_01

9c70679

Conflicts: blackberry/tibb/TiMessageStrings.h

dlifshitz-maca merged commit c063ca4 into Macadamian:blackberry Jul 12, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TIMOB-9874: BlackBerry: implement require() #96

TIMOB-9874: BlackBerry: implement require() #96

dlifshitz-maca commented Jul 9, 2012

alexandergalstyan Jul 10, 2012

dlifshitz-maca Jul 10, 2012

alexandergalstyan commented Jul 10, 2012

alexandergalstyan commented Jul 10, 2012

alexandergalstyan commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

alexandergalstyan commented Jul 11, 2012

alexandergalstyan commented Jul 11, 2012

jpl-mac Jul 11, 2012

Harutyun commented Jul 12, 2012

dlifshitz-maca commented Jul 12, 2012

dlifshitz-maca commented Jul 12, 2012

TIMOB-9874: BlackBerry: implement require() #96

TIMOB-9874: BlackBerry: implement require() #96

Conversation

dlifshitz-maca commented Jul 9, 2012

alexandergalstyan Jul 10, 2012

Choose a reason for hiding this comment

dlifshitz-maca Jul 10, 2012

Choose a reason for hiding this comment

alexandergalstyan commented Jul 10, 2012

alexandergalstyan commented Jul 10, 2012

alexandergalstyan commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

dlifshitz-maca commented Jul 10, 2012

alexandergalstyan commented Jul 11, 2012

alexandergalstyan commented Jul 11, 2012

jpl-mac Jul 11, 2012

Choose a reason for hiding this comment

Harutyun commented Jul 12, 2012

dlifshitz-maca commented Jul 12, 2012

dlifshitz-maca commented Jul 12, 2012