From 7a947fe9d1e610a5ea8ec2016d49f2a8441a4b5e Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sat, 9 Jul 2016 11:31:15 +0200 Subject: [PATCH 1/8] rename v8js_tmpl_t -> v8js_function_tmpl_t --- v8js_class.cc | 20 ++++++++++---------- v8js_class.h | 14 +++++++------- v8js_object_export.cc | 16 ++++++++-------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/v8js_class.cc b/v8js_class.cc index c424f32..62a0263 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -113,21 +113,21 @@ static void v8js_free_storage(zend_object *object) /* {{{ */ c->array_tmpl.~Persistent(); /* Clear persistent call_impl & method_tmpls templates */ - for (std::map::iterator it = c->call_impls.begin(); + for (std::map::iterator it = c->call_impls.begin(); it != c->call_impls.end(); ++it) { // No need to free it->first, as it is stored in c->template_cache and freed below it->second.Reset(); } c->call_impls.~map(); - for (std::map::iterator it = c->method_tmpls.begin(); + for (std::map::iterator it = c->method_tmpls.begin(); it != c->method_tmpls.end(); ++it) { it->second.Reset(); } c->method_tmpls.~map(); /* Clear persistent handles in template cache */ - for (std::map::iterator it = c->template_cache.begin(); + for (std::map::iterator it = c->template_cache.begin(); it != c->template_cache.end(); ++it) { it->second.Reset(); } @@ -158,9 +158,9 @@ static void v8js_free_storage(zend_object *object) /* {{{ */ } c->weak_objects.~map(); - for (std::map::iterator it = c->weak_closures.begin(); + for (std::map::iterator it = c->weak_closures.begin(); it != c->weak_closures.end(); ++it) { - v8js_tmpl_t *persist_tpl_ = it->first; + v8js_function_tmpl_t *persist_tpl_ = it->first; persist_tpl_->Reset(); delete persist_tpl_; it->second.Reset(); @@ -229,13 +229,13 @@ static zend_object* v8js_new(zend_class_entry *ce) /* {{{ */ new(&c->modules_base) std::vector(); new(&c->modules_loaded) std::map; - new(&c->template_cache) std::map(); + new(&c->template_cache) std::map(); new(&c->accessor_list) std::vector(); - new(&c->weak_closures) std::map(); + new(&c->weak_closures) std::map(); new(&c->weak_objects) std::map(); - new(&c->call_impls) std::map(); - new(&c->method_tmpls) std::map(); + new(&c->call_impls) std::map(); + new(&c->method_tmpls) std::map(); new(&c->v8js_v8objects) std::list(); new(&c->script_objects) std::vector(); @@ -589,7 +589,7 @@ static PHP_METHOD(V8Js, __construct) ft = v8::FunctionTemplate::New(isolate, v8js_php_callback, v8::External::New((isolate), method_ptr)); // @fixme add/check Signature v8::Signature::New((isolate), tmpl)); - v8js_tmpl_t *persistent_ft = &c->method_tmpls[method_ptr]; + v8js_function_tmpl_t *persistent_ft = &c->method_tmpls[method_ptr]; persistent_ft->Reset(isolate, ft); } diff --git a/v8js_class.h b/v8js_class.h index 6fb6f83..d94ffa5 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -17,7 +17,7 @@ /* Abbreviate long type names */ -typedef v8::Persistent > v8js_tmpl_t; +typedef v8::Persistent > v8js_function_tmpl_t; typedef v8::Persistent > v8js_persistent_obj_t; /* Forward declarations */ @@ -48,8 +48,8 @@ struct v8js_ctx { bool memory_limit_hit; long average_object_size; - v8js_tmpl_t global_template; - v8js_tmpl_t array_tmpl; + v8js_function_tmpl_t global_template; + v8js_function_tmpl_t array_tmpl; zval module_normaliser; zval module_loader; @@ -57,12 +57,12 @@ struct v8js_ctx { std::vector modules_stack; std::vector modules_base; std::map modules_loaded; - std::map template_cache; + std::map template_cache; std::map weak_objects; - std::map weak_closures; - std::map call_impls; - std::map method_tmpls; + std::map weak_closures; + std::map call_impls; + std::map method_tmpls; std::list v8js_v8objects; diff --git a/v8js_object_export.cc b/v8js_object_export.cc index d733ae2..4a2c425 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -291,10 +291,10 @@ static void v8js_weak_object_callback(const v8::WeakCallbackInfo &d isolate->AdjustAmountOfExternalAllocatedMemory(-ctx->average_object_size); } -static void v8js_weak_closure_callback(const v8::WeakCallbackInfo &data) { +static void v8js_weak_closure_callback(const v8::WeakCallbackInfo &data) { v8::Isolate *isolate = data.GetIsolate(); - v8js_tmpl_t *persist_tpl_ = data.GetParameter(); + v8js_function_tmpl_t *persist_tpl_ = data.GetParameter(); persist_tpl_->Reset(); delete persist_tpl_; @@ -559,7 +559,7 @@ static void v8js_fake_call_impl(const v8::FunctionCallbackInfo& info) v8::Local tmpl = v8::Local::New - (isolate, *reinterpret_cast(self->GetAlignedPointerFromInternalField(0))); + (isolate, *reinterpret_cast(self->GetAlignedPointerFromInternalField(0))); // use v8js_php_callback to actually execute the method v8::Local cb = PHP_V8JS_CALLBACK(isolate, method_ptr, tmpl); uint32_t i, argc = args->Length(); @@ -597,7 +597,7 @@ v8::Local v8js_named_property_callback(v8::Local property zval zobject; ZVAL_OBJ(&zobject, object); - v8js_tmpl_t *tmpl_ptr = reinterpret_cast(self->GetAlignedPointerFromInternalField(0)); + v8js_function_tmpl_t *tmpl_ptr = reinterpret_cast(self->GetAlignedPointerFromInternalField(0)); v8::Local tmpl = v8::Local::New(isolate, *tmpl_ptr); ce = scope = object->ce; @@ -641,7 +641,7 @@ v8::Local v8js_named_property_callback(v8::Local property ft = v8::FunctionTemplate::New(isolate, v8js_fake_call_impl, V8JS_NULL, v8::Signature::New(isolate, tmpl)); - v8js_tmpl_t *persistent_ft = &ctx->call_impls[tmpl_ptr]; + v8js_function_tmpl_t *persistent_ft = &ctx->call_impls[tmpl_ptr]; persistent_ft->Reset(isolate, ft); } v8::Local cb = ft->GetFunction(); @@ -657,7 +657,7 @@ v8::Local v8js_named_property_callback(v8::Local property ft = v8::FunctionTemplate::New(isolate, v8js_php_callback, v8::External::New((isolate), method_ptr), v8::Signature::New((isolate), tmpl)); - v8js_tmpl_t *persistent_ft = &ctx->method_tmpls[method_ptr]; + v8js_function_tmpl_t *persistent_ft = &ctx->method_tmpls[method_ptr]; persistent_ft->Reset(isolate, ft); } ret_value = ft->GetFunction(); @@ -815,7 +815,7 @@ static v8::MaybeLocal v8js_wrap_object(v8::Isolate *isolate, zend_cl { v8js_ctx *ctx = (v8js_ctx *) isolate->GetData(0); v8::Local new_tpl; - v8js_tmpl_t *persist_tpl_; + v8js_function_tmpl_t *persist_tpl_; try { new_tpl = v8::Local::New @@ -836,7 +836,7 @@ static v8::MaybeLocal v8js_wrap_object(v8::Isolate *isolate, zend_cl if (ce == zend_ce_closure) { /* Got a closure, mustn't cache ... */ - persist_tpl_ = new v8js_tmpl_t(isolate, new_tpl); + persist_tpl_ = new v8js_function_tmpl_t(isolate, new_tpl); /* We'll free persist_tpl_ via v8js_weak_closure_callback, below */ new_tpl->InstanceTemplate()->SetCallAsFunctionHandler(v8js_php_callback); } else { From bd730068a27849c768b63e914b433a2b5c6aee22 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sat, 9 Jul 2016 11:31:44 +0200 Subject: [PATCH 2/8] use ObjectTemplate as base for global object --- v8js_class.cc | 8 +++----- v8js_class.h | 3 ++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/v8js_class.cc b/v8js_class.cc index 62a0263..4795c65 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -425,16 +425,14 @@ static PHP_METHOD(V8Js, __construct) /* Create global template for global object */ // Now we are using multiple isolates this needs to be created for every context - v8::Local tpl = v8::FunctionTemplate::New(c->isolate, 0); - - tpl->SetClassName(V8JS_SYM("V8Js")); + v8::Local tpl = v8::ObjectTemplate::New(c->isolate); c->global_template.Reset(isolate, tpl); /* Register builtin methods */ - v8js_register_methods(tpl->InstanceTemplate(), c); + v8js_register_methods(tpl, c); /* Create context */ - v8::Local context = v8::Context::New(isolate, &extension_conf, tpl->InstanceTemplate()); + v8::Local context = v8::Context::New(isolate, &extension_conf, tpl); if (exts) { v8js_free_ext_strarr(exts, exts_count); diff --git a/v8js_class.h b/v8js_class.h index d94ffa5..e05b061 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -18,6 +18,7 @@ /* Abbreviate long type names */ typedef v8::Persistent > v8js_function_tmpl_t; +typedef v8::Persistent > v8js_object_tmpl_t; typedef v8::Persistent > v8js_persistent_obj_t; /* Forward declarations */ @@ -48,7 +49,7 @@ struct v8js_ctx { bool memory_limit_hit; long average_object_size; - v8js_function_tmpl_t global_template; + v8js_object_tmpl_t global_template; v8js_function_tmpl_t array_tmpl; zval module_normaliser; From e4ab07de03a344c8b6adf342b5604668708efe20 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sat, 9 Jul 2016 15:52:45 +0200 Subject: [PATCH 3/8] Add Node.js-style "global" to global scope --- tests/global_object_basic.phpt | 20 ++++++++++++++++++++ v8js_class.cc | 9 +++++---- 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 tests/global_object_basic.phpt diff --git a/tests/global_object_basic.phpt b/tests/global_object_basic.phpt new file mode 100644 index 0000000..862cbef --- /dev/null +++ b/tests/global_object_basic.phpt @@ -0,0 +1,20 @@ +--TEST-- +Test V8Js::executeString : Global scope links global object +--SKIPIF-- + +--FILE-- +executeString($JS); +?> +===EOF=== +--EXPECT-- +string(6) "object" +bool(true) +===EOF=== \ No newline at end of file diff --git a/v8js_class.cc b/v8js_class.cc index 4795c65..db3fcdd 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -425,14 +425,14 @@ static PHP_METHOD(V8Js, __construct) /* Create global template for global object */ // Now we are using multiple isolates this needs to be created for every context - v8::Local tpl = v8::ObjectTemplate::New(c->isolate); - c->global_template.Reset(isolate, tpl); + v8::Local global_template = v8::ObjectTemplate::New(c->isolate); + c->global_template.Reset(isolate, global_template); /* Register builtin methods */ - v8js_register_methods(tpl, c); + v8js_register_methods(global_template, c); /* Create context */ - v8::Local context = v8::Context::New(isolate, &extension_conf, tpl); + v8::Local context = v8::Context::New(isolate, &extension_conf, global_template); if (exts) { v8js_free_ext_strarr(exts, exts_count); @@ -447,6 +447,7 @@ static PHP_METHOD(V8Js, __construct) } context->SetAlignedPointerInEmbedderData(1, c); + context->Global()->Set(context, V8JS_SYM("global"), context->Global()); c->context.Reset(isolate, context); /* Enter context */ From c20c19c126aade8e51bff0643dd25adc8f732a8a Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 12 Nov 2017 15:28:44 +0100 Subject: [PATCH 4/8] add test so that 'this === global' --- tests/global_object_basic.phpt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/global_object_basic.phpt b/tests/global_object_basic.phpt index 862cbef..2f5a16e 100644 --- a/tests/global_object_basic.phpt +++ b/tests/global_object_basic.phpt @@ -8,6 +8,10 @@ Test V8Js::executeString : Global scope links global object $JS = <<< EOT var_dump(typeof global); var_dump(global.var_dump === var_dump); + +// also this is equal to global scope, at least in global execution context +// (i.e. off modules) +var_dump(this === global); EOT; $v8 = new V8Js(); @@ -17,4 +21,5 @@ $v8->executeString($JS); --EXPECT-- string(6) "object" bool(true) -===EOF=== \ No newline at end of file +bool(true) +===EOF=== From f3a46ff833a1fbeaf35a3f82ceccb393cd4c64cd Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 12 Nov 2017 16:14:32 +0100 Subject: [PATCH 5/8] re-use global context for modules + provide module.exports --- tests/commonjs_node_compat_basic.phpt | 41 +++++++++++++++++ v8js_methods.cc | 66 ++++++++++++++------------- 2 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 tests/commonjs_node_compat_basic.phpt diff --git a/tests/commonjs_node_compat_basic.phpt b/tests/commonjs_node_compat_basic.phpt new file mode 100644 index 0000000..ac03757 --- /dev/null +++ b/tests/commonjs_node_compat_basic.phpt @@ -0,0 +1,41 @@ +--TEST-- +Test V8::executeString() : exports/module.exports behaviour +--SKIPIF-- + +--FILE-- +setModuleLoader(function ($moduleName) { + return <<<'EOJS' + var_dump(typeof exports); + var_dump(typeof module.exports); + + // for compatibility both should be linked + var_dump(exports === module.exports); + + exports = { number: 23 }; + module.exports = { number: 42 }; +EOJS + ; +}); + +$v8->executeString(<<<'EOJS' + var result = require('foo'); + + // expect module.exports value to be picked up + var_dump(typeof result); + var_dump(result.number); +EOJS +); + +?> +===EOF=== +--EXPECT-- +string(6) "object" +string(6) "object" +bool(true) +string(6) "object" +int(42) +===EOF=== diff --git a/v8js_methods.cc b/v8js_methods.cc index 21fef8b..69786d6 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -400,24 +400,7 @@ V8JS_METHOD(require) convert_to_string(&module_code); } - // Create a template for the global object and set the built-in global functions - v8::Local global_template = v8::ObjectTemplate::New(isolate); - global_template->Set(V8JS_SYM("print"), v8::FunctionTemplate::New(isolate, V8JS_MN(print)), v8::ReadOnly); - global_template->Set(V8JS_SYM("var_dump"), v8::FunctionTemplate::New(isolate, V8JS_MN(var_dump)), v8::ReadOnly); - global_template->Set(V8JS_SYM("sleep"), v8::FunctionTemplate::New(isolate, V8JS_MN(sleep)), v8::ReadOnly); - global_template->Set(V8JS_SYM("require"), v8::FunctionTemplate::New(isolate, V8JS_MN(require), v8::External::New(isolate, c)), v8::ReadOnly); - - // Add the exports object in which the module can return its API - v8::Local exports_template = v8::ObjectTemplate::New(isolate); - global_template->Set(V8JS_SYM("exports"), exports_template); - - // Add the module object in which the module can have more fine-grained control over what it can return - v8::Local module_template = v8::ObjectTemplate::New(isolate); - module_template->Set(V8JS_SYM("id"), V8JS_STR(normalised_module_id)); - global_template->Set(V8JS_SYM("module"), module_template); - - // Each module gets its own context so different modules do not affect each other - v8::Local context = v8::Local::New(isolate, v8::Context::New(isolate, NULL, global_template)); + v8::Local context = v8::Local::New(isolate, c->context); // Catch JS exceptions v8::TryCatch try_catch(isolate); @@ -429,6 +412,7 @@ V8JS_METHOD(require) // Enter the module context v8::Context::Scope scope(context); + // Set script identifier v8::Local sname = V8JS_STR(normalised_module_id); @@ -438,9 +422,12 @@ V8JS_METHOD(require) return; } - v8::Local source = V8JS_STRL(Z_STRVAL(module_code), static_cast(Z_STRLEN(module_code))); + v8::Local source = V8JS_ZSTR(Z_STR(module_code)); zval_ptr_dtor(&module_code); + source = v8::String::Concat(V8JS_SYM("(function (exports, module) {"), source); + source = v8::String::Concat(source, V8JS_SYM("\n});")); + // Create and compile script v8::Local script = v8::Script::Compile(source, sname); @@ -454,11 +441,29 @@ V8JS_METHOD(require) // Add this module and path to the stack c->modules_stack.push_back(normalised_module_id); - c->modules_base.push_back(normalised_path); - // Run script - script->Run(); + // Run script to evaluate closure + v8::Local module_function = script->Run(); + + // Prepare exports & module object + v8::Local exports = v8::Object::New(isolate); + + v8::Local module = v8::Object::New(isolate); + module->Set(V8JS_SYM("id"), V8JS_STR(normalised_module_id)); + module->Set(V8JS_SYM("exports"), exports); + + if (module_function->IsFunction()) { + v8::Local *jsArgv = static_cast *>(alloca(2 * sizeof(v8::Local))); + new(&jsArgv[0]) v8::Local; + jsArgv[0] = exports; + + new(&jsArgv[1]) v8::Local; + jsArgv[1] = module; + + // actually call the module + v8::Local::Cast(module_function)->Call(V8JS_GLOBAL(isolate), 2, jsArgv); + } // Remove this module and path from the stack c->modules_stack.pop_back(); @@ -466,6 +471,12 @@ V8JS_METHOD(require) efree(normalised_path); + if (!module_function->IsFunction()) { + info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Wrapped module script failed to return function"))); + efree(normalised_module_id); + return; + } + // Script possibly terminated, return immediately if (!try_catch.CanContinue()) { info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module script compile failed"))); @@ -485,15 +496,8 @@ V8JS_METHOD(require) // Cache the module so it doesn't need to be compiled and run again // Ensure compatibility with CommonJS implementations such as NodeJS by playing nicely with module.exports and exports - if (context->Global()->Has(V8JS_SYM("module")) - && context->Global()->Get(V8JS_SYM("module"))->IsObject() - && context->Global()->Get(V8JS_SYM("module"))->ToObject()->Has(V8JS_SYM("exports")) - && context->Global()->Get(V8JS_SYM("module"))->ToObject()->Get(V8JS_SYM("exports"))->IsObject()) { - // If module.exports has been set then we cache this arbitrary value... - newobj = context->Global()->Get(V8JS_SYM("module"))->ToObject()->Get(V8JS_SYM("exports"))->ToObject(); - } else { - // ...otherwise we cache the exports object itself - newobj = context->Global()->Get(V8JS_SYM("exports"))->ToObject(); + if (module->Has(V8JS_SYM("exports"))) { + newobj = module->Get(V8JS_SYM("exports"))->ToObject(); } c->modules_loaded[normalised_module_id].Reset(isolate, newobj); From 384ec9b1b6a73940ef7c9ad8a424a7248ba2872f Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 12 Nov 2017 16:20:24 +0100 Subject: [PATCH 6/8] use "this = module.exports" for modules --- tests/commonjs_node_compat_001.phpt | 28 +++++++++++++++++++++++++++ tests/commonjs_node_compat_basic.phpt | 2 +- v8js_methods.cc | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/commonjs_node_compat_001.phpt diff --git a/tests/commonjs_node_compat_001.phpt b/tests/commonjs_node_compat_001.phpt new file mode 100644 index 0000000..36738a5 --- /dev/null +++ b/tests/commonjs_node_compat_001.phpt @@ -0,0 +1,28 @@ +--TEST-- +Test V8Js::setModuleLoader : this === module.exports +--SKIPIF-- + +--FILE-- +setModuleLoader(function ($moduleName) { + return <<<'EOJS' + var_dump(this === global); + var_dump(this === module.exports); +EOJS + ; +}); + +$v8->executeString(<<<'EOJS' + var result = require('foo'); +EOJS +); + +?> +===EOF=== +--EXPECT-- +bool(false) +bool(true) +===EOF=== diff --git a/tests/commonjs_node_compat_basic.phpt b/tests/commonjs_node_compat_basic.phpt index ac03757..6da4094 100644 --- a/tests/commonjs_node_compat_basic.phpt +++ b/tests/commonjs_node_compat_basic.phpt @@ -1,5 +1,5 @@ --TEST-- -Test V8::executeString() : exports/module.exports behaviour +Test V8Js::setModuleLoader : exports/module.exports behaviour --SKIPIF-- --FILE-- diff --git a/v8js_methods.cc b/v8js_methods.cc index 69786d6..ac69d64 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -462,7 +462,7 @@ V8JS_METHOD(require) jsArgv[1] = module; // actually call the module - v8::Local::Cast(module_function)->Call(V8JS_GLOBAL(isolate), 2, jsArgv); + v8::Local::Cast(module_function)->Call(exports, 2, jsArgv); } // Remove this module and path from the stack From 1c7e355937d4ced59571b7e9420970bc5c0e143f Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 12 Nov 2017 16:25:59 +0100 Subject: [PATCH 7/8] allow modules to return arbitrary values --- tests/commonjs_node_compat_002.phpt | 29 +++++++++++++++++++++++++++++ v8js_class.cc | 4 ++-- v8js_class.h | 3 ++- v8js_methods.cc | 6 +++--- 4 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 tests/commonjs_node_compat_002.phpt diff --git a/tests/commonjs_node_compat_002.phpt b/tests/commonjs_node_compat_002.phpt new file mode 100644 index 0000000..2ee279f --- /dev/null +++ b/tests/commonjs_node_compat_002.phpt @@ -0,0 +1,29 @@ +--TEST-- +Test V8Js::setModuleLoader : modules can return arbitrary values +--SKIPIF-- + +--FILE-- +setModuleLoader(function ($moduleName) { + return <<<'EOJS' + module.exports = 23; +EOJS + ; +}); + +$v8->executeString(<<<'EOJS' + var result = require('foo'); + var_dump(typeof result); + var_dump(result); +EOJS +); + +?> +===EOF=== +--EXPECT-- +string(6) "number" +int(23) +===EOF=== diff --git a/v8js_class.cc b/v8js_class.cc index db3fcdd..4aa4c54 100644 --- a/v8js_class.cc +++ b/v8js_class.cc @@ -182,7 +182,7 @@ static void v8js_free_storage(zend_object *object) /* {{{ */ c->script_objects.~vector(); /* Clear persistent handles in module cache */ - for (std::map::iterator it = c->modules_loaded.begin(); + for (std::map::iterator it = c->modules_loaded.begin(); it != c->modules_loaded.end(); ++it) { efree(it->first); it->second.Reset(); @@ -227,7 +227,7 @@ static zend_object* v8js_new(zend_class_entry *ce) /* {{{ */ new(&c->modules_stack) std::vector(); new(&c->modules_base) std::vector(); - new(&c->modules_loaded) std::map; + new(&c->modules_loaded) std::map; new(&c->template_cache) std::map(); new(&c->accessor_list) std::vector(); diff --git a/v8js_class.h b/v8js_class.h index e05b061..4886754 100644 --- a/v8js_class.h +++ b/v8js_class.h @@ -20,6 +20,7 @@ typedef v8::Persistent > v8js_function_tmpl_t; typedef v8::Persistent > v8js_object_tmpl_t; typedef v8::Persistent > v8js_persistent_obj_t; +typedef v8::Persistent > v8js_persistent_value_t; /* Forward declarations */ struct v8js_v8object; @@ -57,7 +58,7 @@ struct v8js_ctx { std::vector modules_stack; std::vector modules_base; - std::map modules_loaded; + std::map modules_loaded; std::map template_cache; std::map weak_objects; diff --git a/v8js_methods.cc b/v8js_methods.cc index ac69d64..6fb4c03 100644 --- a/v8js_methods.cc +++ b/v8js_methods.cc @@ -336,7 +336,7 @@ V8JS_METHOD(require) // If we have already loaded and cached this module then use it if (c->modules_loaded.count(normalised_module_id) > 0) { - v8::Persistent newobj; + v8::Persistent newobj; newobj.Reset(isolate, c->modules_loaded[normalised_module_id]); info.GetReturnValue().Set(newobj); @@ -492,12 +492,12 @@ V8JS_METHOD(require) return; } - v8::Local newobj; + v8::Local newobj; // Cache the module so it doesn't need to be compiled and run again // Ensure compatibility with CommonJS implementations such as NodeJS by playing nicely with module.exports and exports if (module->Has(V8JS_SYM("exports"))) { - newobj = module->Get(V8JS_SYM("exports"))->ToObject(); + newobj = module->Get(V8JS_SYM("exports")); } c->modules_loaded[normalised_module_id].Reset(isolate, newobj); From 6a2f53a9f196385fe692caf9134a6678c65806b6 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 12 Nov 2017 16:27:50 +0100 Subject: [PATCH 8/8] test "delete module.exports" --- tests/commonjs_node_compat_003.phpt | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/commonjs_node_compat_003.phpt diff --git a/tests/commonjs_node_compat_003.phpt b/tests/commonjs_node_compat_003.phpt new file mode 100644 index 0000000..240aff4 --- /dev/null +++ b/tests/commonjs_node_compat_003.phpt @@ -0,0 +1,27 @@ +--TEST-- +Test V8Js::setModuleLoader : delete module.exports yields undefined +--SKIPIF-- + +--FILE-- +setModuleLoader(function ($moduleName) { + return <<<'EOJS' + delete module.exports; +EOJS + ; +}); + +$v8->executeString(<<<'EOJS' + var result = require('foo'); + var_dump(typeof result); +EOJS +); + +?> +===EOF=== +--EXPECT-- +string(9) "undefined" +===EOF===