Merge pull request #141 from NativeScript/plamen5kov/refactor_require

Plamen5kov/refactor require

Author: Plamen5kov

src/jni/Constants.h

@@ -10,6 +10,8 @@ class Constants
 
 	const static char CLASS_NAME_LOCATION_SEPARATOR = '_';
 
+	static std::string APP_ROOT_FOLDER_PATH;
+
 private:
 	Constants() {}
 };

src/jni/MetadataNode.cpp

@@ -1201,12 +1201,19 @@ bool MetadataNode::GetExtendLocation(string& extendLocation)
 			}
 
 			string srcFileName = ConvertToString(scriptName);
-			std::replace(srcFileName.begin(), srcFileName.end(), '/', '_');
-			std::replace(srcFileName.begin(), srcFileName.end(), '.', '_');
+
+			string hardcodedPathToSkip = Constants::APP_ROOT_FOLDER_PATH;
+
+			int startIndex = hardcodedPathToSkip.length();
+			int strToTakeLen = (srcFileName.length() - startIndex - 3); // 3 refers to .js at the end of file name
+			string fullPathToFile = srcFileName.substr(startIndex, strToTakeLen);
+
+			std::replace(fullPathToFile.begin(), fullPathToFile.end(), '/', '_');
+			std::replace(fullPathToFile.begin(), fullPathToFile.end(), '.', '_');
 			int lineNumber = frame->GetLineNumber();
 			if (lineNumber < 0)
 			{
-				extendLocationStream << srcFileName.c_str() << " unkown line number";
+				extendLocationStream << fullPathToFile.c_str() << " unkown line number";
 				extendLocation = extendLocationStream.str();
 				return false;
 			}
@@ -1219,13 +1226,13 @@ bool MetadataNode::GetExtendLocation(string& extendLocation)
 			int column = frame->GetColumn();
 			if (column < 0)
 			{
-				extendLocationStream << srcFileName.c_str() << " line:" << lineNumber << " unkown column number";
+				extendLocationStream << fullPathToFile.c_str() << " line:" << lineNumber << " unkown column number";
 				extendLocation = extendLocationStream.str();
 				return false;
 			}
 
 
-			extendLocationStream << "f" << srcFileName.c_str() << "_l" << lineNumber << "_c" << column << "__";
+			extendLocationStream << "f" << fullPathToFile.c_str() << "_l" << lineNumber << "_c" << column << "__";
 			//DEBUG_WRITE("EXTEND_LOCATION %s", extendLocationStream.str().c_str());
 		}
 	}

src/jni/NativeScriptRuntime.cpp

@@ -26,7 +26,6 @@ using namespace v8;
 using namespace std;
 using namespace tns;
 
-// init
 void NativeScriptRuntime::Init(JavaVM *jvm, ObjectManager *objectManager)
 {
 	NativeScriptRuntime::jvm = jvm;
@@ -735,6 +734,103 @@ void NativeScriptRuntime::CreateGlobalCastFunctions(const Handle<ObjectTemplate>
 }
 
 
+void NativeScriptRuntime::CompileAndRun(string modulePath, bool& hasError, Handle<Object>& moduleObj, bool isBootstrapCall)
+{
+	auto isolate = Isolate::GetCurrent();
+
+	Local < Value > exportObj = Object::New(isolate);
+	auto tmpExportObj = new Persistent<Object>(isolate, exportObj.As<Object>());
+	loadedModules.insert(make_pair(modulePath, tmpExportObj));
+
+	TryCatch tc;
+
+	auto scriptText = Require::LoadModule(modulePath);
+
+	DEBUG_WRITE("Compiling script (module %s)", modulePath.c_str());
+	Local < String > fullRequiredModulePath = ConvertToV8String(modulePath);
+	auto script = Script::Compile(scriptText, fullRequiredModulePath);
+	DEBUG_WRITE("Compiled script (module %s)", modulePath.c_str());
+
+	if (ExceptionUtil::GetInstance()->HandleTryCatch(tc, "Script " + modulePath + " contains compilation errors!"))
+	{
+		loadedModules.erase(modulePath);
+		tmpExportObj->Reset();
+		delete tmpExportObj;
+		hasError = true;
+	}
+	else if (script.IsEmpty())
+	{
+		//think about more descriptive message -> [script_name] was empty
+		DEBUG_WRITE("%s was empty", modulePath.c_str());
+	}
+	else
+	{
+		DEBUG_WRITE("Running script (module %s)", modulePath.c_str());
+
+		TryCatch tcRequire;
+
+		Local < Function > f = script->Run().As<Function>();
+		if (ExceptionUtil::GetInstance()->HandleTryCatch(tc))
+		{
+			DEBUG_WRITE("Exception was handled in java code");
+		}
+
+		//this is done so the initial bootstrap function is persistent (and to keep old logic)
+		if(isBootstrapCall) {
+			auto persistentAppModule = new Persistent<Object>(isolate, f);
+		}
+
+		auto result = f->Call(Object::New(isolate), 1, &exportObj);
+		if(ExceptionUtil::GetInstance()->HandleTryCatch(tc))
+		{
+			DEBUG_WRITE("Exception was handled in java code");
+		}
+		else
+		{
+			moduleObj = result.As<Object>();
+		}
+
+		// introducing isBootstrapCall in order to save the flow as it was (latter on we can think about including the following code for the bootstrap (initial) call
+		if(!isBootstrapCall) {
+			DEBUG_WRITE("After Running script (module %s)", modulePath.c_str());
+
+			if (ExceptionUtil::GetInstance()->HandleTryCatch(tcRequire))
+			{
+				loadedModules.erase(modulePath);
+				tmpExportObj->Reset();
+				delete tmpExportObj;
+				hasError = true;
+				tcRequire.ReThrow();
+			}
+			else
+			{
+				if (moduleObj.IsEmpty())
+				{
+					auto objectTemplate = ObjectTemplate::New();
+					moduleObj = objectTemplate->NewInstance();
+				}
+
+				DEBUG_WRITE("Script completed (module %s)", modulePath.c_str());
+
+				if (!moduleObj->StrictEquals(exportObj))
+				{
+					loadedModules.erase(modulePath);
+					tmpExportObj->Reset();
+					delete tmpExportObj;
+
+					auto persistentModuleObject = new Persistent<Object>(isolate, moduleObj.As<Object>());
+
+					loadedModules.insert(make_pair(modulePath, persistentModuleObject));
+				}
+			}
+		}
+	}
+	if (tc.HasCaught())
+	{
+		tc.ReThrow();
+	}
+}
+
 void NativeScriptRuntime::RequireCallback(const v8::FunctionCallbackInfo<v8::Value>& args)
 {
 	SET_PROFILER_FRAME();
@@ -782,67 +878,7 @@ void NativeScriptRuntime::RequireCallback(const v8::FunctionCallbackInfo<v8::Val
 
 	if (it == loadedModules.end())
 	{
-		Local<Value> exportObj = Object::New(isolate);
-		auto tmpExportObj = new Persistent<Object>(isolate, exportObj.As<Object>());
-		loadedModules.insert(make_pair(modulePath, tmpExportObj));
-
-		TryCatch tc;
-		auto scriptText = Require::LoadModule(modulePath);
-
-		DEBUG_WRITE("Compiling script (module %s)", moduleName.c_str());
-		auto script = Script::Compile(scriptText, args[0].As<String>());
-		DEBUG_WRITE("Compiled script (module %s)", moduleName.c_str());
-
-		if(ExceptionUtil::GetInstance()->HandleTryCatch(tc)){
-			loadedModules.erase(modulePath);
-			tmpExportObj->Reset();
-			delete tmpExportObj;
-			hasError = true;
-		}
-		else {
-			DEBUG_WRITE("Running script (module %s)", moduleName.c_str());
-
-			TryCatch tcRequire;
-
-			Local<Function> f = script->Run().As<Function>();
-			auto result = f->Call(Object::New(isolate), 1, &exportObj);
-
-			moduleObj = result.As<Object>();
-
-			DEBUG_WRITE("After Running script (module %s)", moduleName.c_str());
-
-			if(ExceptionUtil::GetInstance()->HandleTryCatch(tcRequire)){
-				loadedModules.erase(modulePath);
-				tmpExportObj->Reset();
-				delete tmpExportObj;
-				hasError = true;
-				tcRequire.ReThrow();
-			}
-			else {
-				if (moduleObj.IsEmpty())
-				{
-					auto objectTemplate = ObjectTemplate::New();
-					moduleObj = objectTemplate->NewInstance();
-				}
-
-				DEBUG_WRITE("Script completed (module %s)", moduleName.c_str());
-
-				if (!moduleObj->StrictEquals(exportObj))
-				{
-					loadedModules.erase(modulePath);
-					tmpExportObj->Reset();
-					delete tmpExportObj;
-
-					auto persistentModuleObject = new Persistent<Object>(isolate, moduleObj.As<Object>());
-
-					loadedModules.insert(make_pair(modulePath, persistentModuleObject));
-				}
-			}
-		}
-		if (tc.HasCaught())
-		{
-			tc.ReThrow();
-		}
+		CompileAndRun(modulePath, hasError, moduleObj, false/*is bootstrap call*/);
 	}
 	else
 	{

src/jni/NativeScriptRuntime.h

@@ -88,6 +88,8 @@ namespace tns
 
 		static void CreateTopLevelNamespaces(const v8::Handle<v8::Object>& global);
 
+		static void CompileAndRun(std::string modulePath, bool& hasError, v8::Handle<v8::Object>& moduleObj, bool isBootstrapCall);
+
 		static MetadataTreeNode *metadataRoot;
 
 		static jclass PlatformClass;

src/jni/com_tns_Platform.cpp

@@ -38,6 +38,7 @@ int count = 0;
 Context::Scope *context_scope = nullptr;
 bool tns::LogEnabled = true;
 Isolate *g_isolate = nullptr;
+std::string Constants::APP_ROOT_FOLDER_PATH = "";
 
 ObjectManager *g_objectManager = nullptr;
 
@@ -177,78 +178,29 @@ extern "C" void Java_com_tns_Platform_initNativeScript(JNIEnv *_env, jobject obj
 	PrepareV8Runtime(isolate, env, filesPath, packageName);
 
 	NativeScriptRuntime::APP_FILES_DIR = ArgConverter::jstringToString(filesPath);
+	Constants::APP_ROOT_FOLDER_PATH = NativeScriptRuntime::APP_FILES_DIR + "/app/";
 }
 
-extern "C" void Java_com_tns_Platform_runNativeScript(JNIEnv *_env, jobject obj, jstring appModuleName, jstring appCode)
+extern "C" void Java_com_tns_Platform_runNativeScript(JNIEnv *_env, jobject obj, jstring appModuleName)
 {
 	JEnv env(_env);
-
 	auto isolate = g_isolate;
 	Isolate::Scope isolate_scope(isolate);
 
-	TryCatch tc;
-
 	HandleScope handleScope(isolate);
-	auto context = Local<Context>::New(isolate, *PrimaryContext);
-
-	jstring retval;
-	jboolean isCopy;
-
-	const char* code = env.GetStringUTFChars(appCode, &isCopy);
-
-	auto cmd = ConvertToV8String(code);
-
-	env.ReleaseStringUTFChars(appCode, code);
-
-	DEBUG_WRITE("Compiling script");
 
-	auto moduleName = ArgConverter::jstringToV8String(appModuleName);
+	Handle<Object> moduleObject;
+	string modulePath = ArgConverter::jstringToString(appModuleName);
+	bool hasError = false;
 
-	auto script = Script::Compile(cmd, moduleName);
-
-	DEBUG_WRITE("Compile script");
-
-	if (ExceptionUtil::GetInstance()->HandleTryCatch(tc, "Bootstrap script has error(s)."))
-	{
-		DEBUG_WRITE("Exception was handled in java code");
-	}
-	else if (script.IsEmpty())
-	{
-		DEBUG_WRITE("Bootstrap was empty");
-	}
-	else
-	{
-		DEBUG_WRITE("Running script");
-
-		auto appModuleObj = script->Run();
-		if (ExceptionUtil::GetInstance()->HandleTryCatch(tc))
-		{
-			DEBUG_WRITE("Exception was handled in java code");
-		}
-		else if (!appModuleObj.IsEmpty() && appModuleObj->IsFunction())
-		{
-			auto moduleFunc = appModuleObj.As<Function>();
-			Handle<Value> exportsObj = Object::New(isolate);
-			auto thiz = Object::New(isolate);
-			auto res = moduleFunc->Call(thiz, 1, &exportsObj);
-
-			if(ExceptionUtil::GetInstance()->HandleTryCatch(tc))
-			{
-				DEBUG_WRITE("Exception was handled in java code");
-			}
-			else
-			{
-				// TODO: Why do we need this line?
-				auto persistentAppModuleObject = new Persistent<Object>(Isolate::GetCurrent(), appModuleObj.As<Object>());
-			}
-		}
-		else
-		{
-			ExceptionUtil::GetInstance()->ThrowExceptionToJava(tc, "Error running NativeScript bootstrap code.");
-		}
-	}
+	NativeScriptRuntime::CompileAndRun(modulePath, hasError, moduleObject, true /*is bootstrap call*/);
 
-	//NativeScriptRuntime::loadedModules.insert(make_pair(appModuleName, persistentAppModuleObject));
+	/*
+	 * moduleObject (export module) can be set to js variable but currently we start the script implicitly without returning the moduleObject (just calling it)
+	 *
+	 * we can do something like
+	 * var appModule = require("app"); //but currently we call the appModule only once Platform.run() and no one else has access to it
+	 */
 
 	//DEBUG_WRITE("Forcing V8 garbage collection");
 	//while (!V8::IdleNotification());