From 1a8adb1588bc785f8fd8586d66f5e6721598ad55 Mon Sep 17 00:00:00 2001 From: Caio Lima Date: Mon, 3 Aug 2026 09:00:22 -0300 Subject: [PATCH 1/3] deps: V8: backport e7785934cd73 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [api][module] Return a Promise from Module::Evaluate() Module evaluation always produces a Promise, but the public API exposed the result as MaybeLocal, and for synthetic modules it could even hand back the raw, non-Promise value returned by the embedder's evaluation steps. Embedders therefore had to defensively cast the result, and the actual contract was invisible in the type system. This CL makes the contract explicit: - SyntheticModule::Evaluate() now returns the top-level capability Promise instead of the raw value produced by the evaluation steps, so the completion value is always a Promise, matching source text modules and the documented behavior. - SyntheticModule::Evaluate() nows CHECK if callback result from EvaluationSteps is a Promise. The reasoning behind it is that the behavior for an embedder that relies on non-promise result was already inconsistent, given the first call would return the result form callback (an arbitrary Local), but subsequent calls for module->Evaluate() would return the capability with `undefined` as result. - Module::Evaluate() and their variant now returns `MaybeDirectHandle` to be more explicit by the return type. Bug: 531396274 Change-Id: Ifbfed4f849bb47d8db1e022bfb750d9ad2d6308b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8131138 Reviewed-by: Camillo Bruni Commit-Queue: Caio Lima Reviewed-by: Olivier Flückiger Cr-Commit-Position: refs/heads/main@{#109010} Refs: https://github.com/v8/v8/commit/e7785934cd7355fe813660140abd73f4d4886290 Co-authored-by: Caio Lima --- common.gypi | 2 +- deps/v8/include/v8-script.h | 4 +++- deps/v8/src/api/api.cc | 2 +- deps/v8/src/objects/module.cc | 15 ++++++--------- deps/v8/src/objects/module.h | 2 +- deps/v8/src/objects/source-text-module.cc | 4 ++-- deps/v8/src/objects/source-text-module.h | 2 +- deps/v8/src/objects/synthetic-module.cc | 23 +++++------------------ deps/v8/src/objects/synthetic-module.h | 2 +- deps/v8/test/cctest/test-api.cc | 22 ++++++++++++++++++---- 10 files changed, 39 insertions(+), 39 deletions(-) diff --git a/common.gypi b/common.gypi index 0b01ec8c49fe..45b4ddf2faa2 100644 --- a/common.gypi +++ b/common.gypi @@ -42,7 +42,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.28', + 'v8_embedder_string': '-node.29', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-script.h b/deps/v8/include/v8-script.h index c3a2274d4333..875c220e3c36 100644 --- a/deps/v8/include/v8-script.h +++ b/deps/v8/include/v8-script.h @@ -280,7 +280,7 @@ class V8_EXPORT Module : public Data { * * If IsGraphAsync() is false, the returned Promise is settled. */ - V8_WARN_UNUSED_RESULT MaybeLocal Evaluate(Local context); + V8_WARN_UNUSED_RESULT MaybeLocal Evaluate(Local context); /** * Returns the namespace object of this module. @@ -336,6 +336,8 @@ class V8_EXPORT Module : public Data { * exception was thrown) and return an empy MaybeLocal to indicate falure * (where an exception was thrown). */ + // TODO(caiolima): Change this to `MaybeLocal` given it's expected a + // Promise as return. using SyntheticModuleEvaluationSteps = MaybeLocal (*)(Local context, Local module); diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index fbd628370c0b..446f1e2794f9 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -2398,7 +2398,7 @@ Maybe Module::InstantiateModule(Local context, return Just(true); } -MaybeLocal Module::Evaluate(Local context) { +MaybeLocal Module::Evaluate(Local context) { auto i_isolate = i::Isolate::Current(); TRACE_EVENT_CALL_STATS_SCOPED(i_isolate, "v8", "V8.Execute"); EnterV8Scope api_scope{i_isolate, context, diff --git a/deps/v8/src/objects/module.cc b/deps/v8/src/objects/module.cc index 147220347763..557decca5225 100644 --- a/deps/v8/src/objects/module.cc +++ b/deps/v8/src/objects/module.cc @@ -265,8 +265,8 @@ bool Module::FinishInstantiate(Isolate* isolate, Handle module, } } -MaybeDirectHandle Module::Evaluate(Isolate* isolate, - Handle module) { +MaybeDirectHandle Module::Evaluate(Isolate* isolate, + Handle module) { #ifdef DEBUG PrintStatusMessage(*module, "Evaluating module "); #endif // DEBUG @@ -492,16 +492,13 @@ void JSDeferredModuleNamespace::EvaluateModuleSync( return; } - MaybeDirectHandle maybe_result = Module::Evaluate(isolate, module); - DirectHandle result; - if (!maybe_result.ToHandle(&result)) { + MaybeDirectHandle maybe_result = Module::Evaluate(isolate, module); + DirectHandle promise; + if (!maybe_result.ToHandle(&promise)) { return; } - // If there's a result, it needs to be a promise with either Reject or - // Fulfilled status. - DCHECK(IsJSPromise(*result)); - DirectHandle promise = Cast(result); + // The result is always the module's top-level capability promise. // 5. If promise.[[PromiseState]] is rejected, then if (promise->status() == Promise::kRejected) { // a. If promise.[[PromiseIsHandled]] is false, perform diff --git a/deps/v8/src/objects/module.h b/deps/v8/src/objects/module.h index 30e9af6a1dc7..80d23a4e0542 100644 --- a/deps/v8/src/objects/module.h +++ b/deps/v8/src/objects/module.h @@ -74,7 +74,7 @@ class Module : public TorqueGeneratedModule { const UserResolveCallbacks& callbacks); // Implementation of spec operation ModuleEvaluation. - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, Handle module); // Get the namespace object for [module]. If it doesn't exist yet, it is diff --git a/deps/v8/src/objects/source-text-module.cc b/deps/v8/src/objects/source-text-module.cc index 101373b83c61..7c5807616355 100644 --- a/deps/v8/src/objects/source-text-module.cc +++ b/deps/v8/src/objects/source-text-module.cc @@ -886,8 +886,8 @@ bool SourceTextModule::MaybeHandleEvaluationException( return false; } -// ES#sec-moduleevaluation -MaybeDirectHandle SourceTextModule::Evaluate( +// https://tc39.es/ecma262/#sec-moduleevaluation +MaybeDirectHandle SourceTextModule::Evaluate( Isolate* isolate, Handle module) { CHECK(module->status() == kLinked || module->status() == kEvaluatingAsync || module->status() == kEvaluated); diff --git a/deps/v8/src/objects/source-text-module.h b/deps/v8/src/objects/source-text-module.h index 93d9f8f074b7..7be59e5b37a6 100644 --- a/deps/v8/src/objects/source-text-module.h +++ b/deps/v8/src/objects/source-text-module.h @@ -192,7 +192,7 @@ class SourceTextModule AvailableAncestorsSet* exec_list); // Implementation of spec concrete method Evaluate. - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, Handle module); // Implementation of spec abstract operation InnerModuleEvaluation. diff --git a/deps/v8/src/objects/synthetic-module.cc b/deps/v8/src/objects/synthetic-module.cc index fd791dfbe0c1..c80ce99fa73e 100644 --- a/deps/v8/src/objects/synthetic-module.cc +++ b/deps/v8/src/objects/synthetic-module.cc @@ -105,7 +105,7 @@ bool SyntheticModule::FinishInstantiate(Isolate* isolate, // Implements Synthetic Module Record's Evaluate concrete method: // https://heycam.github.io/webidl/#smr-evaluate -MaybeDirectHandle SyntheticModule::Evaluate( +MaybeDirectHandle SyntheticModule::Evaluate( Isolate* isolate, DirectHandle module) { module->SetStatus(kEvaluating); @@ -117,29 +117,16 @@ MaybeDirectHandle SyntheticModule::Evaluate( Utils::ToLocal(Cast(module))) .ToLocal(&result)) { module->RecordError(isolate, isolate->exception()); - return MaybeDirectHandle(); + return MaybeDirectHandle(); } module->SetStatus(kEvaluated); DirectHandle result_from_callback = Utils::OpenDirectHandle(*result); - - DirectHandle capability; - if (IsJSPromise(*result_from_callback)) { - capability = Cast(result_from_callback); - } else { - // The host's evaluation steps should have returned a resolved Promise, - // but as an allowance to hosts that have not yet finished the migration - // to top-level await, create a Promise if the callback result didn't give - // us one. - capability = isolate->factory()->NewJSPromise(); - JSPromise::Resolve(capability, isolate->factory()->undefined_value()) - .ToHandleChecked(); - } - + CHECK(IsJSPromise(*result_from_callback)); + DirectHandle capability = Cast(result_from_callback); module->set_top_level_capability(*capability); - - return result_from_callback; + return capability; } } // namespace internal diff --git a/deps/v8/src/objects/synthetic-module.h b/deps/v8/src/objects/synthetic-module.h index 03185dc16674..3ea24be032b0 100644 --- a/deps/v8/src/objects/synthetic-module.h +++ b/deps/v8/src/objects/synthetic-module.h @@ -60,7 +60,7 @@ class SyntheticModule static V8_WARN_UNUSED_RESULT bool FinishInstantiate( Isolate* isolate, DirectHandle module); - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, DirectHandle module); TQ_OBJECT_CONSTRUCTORS(SyntheticModule) diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index ba7e079f411c..910a18c61ff3 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -24711,7 +24711,11 @@ static int synthetic_module_callback_count; v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( Local context, Local module) { synthetic_module_callback_count++; - return v8::Undefined(reinterpret_cast(CcTest::isolate())); + // Synthetic module evaluation steps must return a Promise. + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); } v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackFail( @@ -24727,7 +24731,11 @@ v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackSetExport( Maybe set_export_result = module->SetSyntheticModuleExport( CcTest::isolate(), v8_str("test_export"), v8_num(42)); CHECK(set_export_result.FromJust()); - return v8::Undefined(reinterpret_cast(CcTest::isolate())); + // Synthetic module evaluation steps must return a Promise. + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); } namespace { @@ -25058,7 +25066,10 @@ TEST(SyntheticModuleEvaluationStepsNoThrow) { context, export_names, SyntheticModuleEvaluationStepsCallback); CHECK_EQ(synthetic_module_callback_count, 0); Local completion_value = module->Evaluate(context).ToLocalChecked(); - CHECK(completion_value->IsUndefined()); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); + CHECK(promise->Result()->IsUndefined()); CHECK_EQ(synthetic_module_callback_count, 1); CHECK_EQ(module->GetStatus(), Module::kEvaluated); } @@ -25116,7 +25127,10 @@ TEST(SyntheticModuleEvaluationStepsSetExport) { CHECK(IsUndefined(test_export_cell->value())); Local completion_value = module->Evaluate(context).ToLocalChecked(); - CHECK(completion_value->IsUndefined()); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); + CHECK(promise->Result()->IsUndefined()); CHECK_EQ(42, i::Object::NumberValue(test_export_cell->value())); CHECK_EQ(module->GetStatus(), Module::kEvaluated); } From dc2deb3aebf549201e96566d319563d932f177c0 Mon Sep 17 00:00:00 2001 From: Caio Lima Date: Thu, 13 Aug 2026 12:01:19 -0300 Subject: [PATCH 2/3] deps: V8: backport 970d651e4f99 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [api][module] Type-check synthetic module evaluation steps Synthetic module evaluation steps are required to return a Promise, which becomes the module's top-level capability. Their signature returned a MaybeLocal though, so that requirement was only enforced by a CHECK in SyntheticModule::Evaluate(). SyntheticModuleEvaluationSteps now returns a MaybeLocal, with a matching CreateSyntheticModule() overload. The old signature remains available as LegacySyntheticModuleEvaluationSteps so that embedders can be migrated in a separate CL; it will be deprecated and then removed once embedders migrate. d8 and the existing tests move to the Promise-returning version, with one cctest checking the legacy version. Bug: 545375591 Change-Id: Id55db730678455f81394bf8a68f66f93d22f5547 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8223168 Reviewed-by: Olivier Flückiger Reviewed-by: Igor Sheludko Commit-Queue: Caio Lima Cr-Commit-Position: refs/heads/main@{#109273} Refs: https://github.com/v8/v8/commit/970d651e4f9975430f9f4cf7e8c45ba7566f52a2 Co-authored-by: Caio Lima --- common.gypi | 2 +- deps/v8/include/v8-script.h | 21 ++++++- deps/v8/src/api/api.cc | 27 ++++++++ deps/v8/src/d8/d8.cc | 2 +- deps/v8/src/d8/d8.h | 5 +- deps/v8/src/objects/synthetic-module.cc | 12 ++++ deps/v8/test/cctest/test-api.cc | 61 ++++++++++++++++--- .../unittests/objects/modules-unittest.cc | 6 +- 8 files changed, 118 insertions(+), 18 deletions(-) diff --git a/common.gypi b/common.gypi index 45b4ddf2faa2..377377b65c5d 100644 --- a/common.gypi +++ b/common.gypi @@ -42,7 +42,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.29', + 'v8_embedder_string': '-node.30', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-script.h b/deps/v8/include/v8-script.h index 875c220e3c36..f7a97a0fc370 100644 --- a/deps/v8/include/v8-script.h +++ b/deps/v8/include/v8-script.h @@ -336,9 +336,16 @@ class V8_EXPORT Module : public Data { * exception was thrown) and return an empy MaybeLocal to indicate falure * (where an exception was thrown). */ - // TODO(caiolima): Change this to `MaybeLocal` given it's expected a - // Promise as return. using SyntheticModuleEvaluationSteps = + MaybeLocal (*)(Local context, Local module); + + /* + * Deprecated version of SyntheticModuleEvaluationSteps: the returned value is + * still required to be a Promise, but that is only enforced at runtime. + */ + // TODO(https://crbug.com/545375591): Remove once all embedders return a + // MaybeLocal. + using LegacySyntheticModuleEvaluationSteps = MaybeLocal (*)(Local context, Local module); /** @@ -353,6 +360,16 @@ class V8_EXPORT Module : public Data { const MemorySpan>& export_names, SyntheticModuleEvaluationSteps evaluation_steps); + // TODO(https://crbug.com/545375591): Advance to V8_DEPRECATED and then remove + // this overload once all embedders have been migrated to the one above. + V8_DEPRECATE_SOON( + "Use the CreateSyntheticModule overload whose evaluation_steps return a " + "MaybeLocal") + static Local CreateSyntheticModule( + Isolate* isolate, Local module_name, + const MemorySpan>& export_names, + LegacySyntheticModuleEvaluationSteps evaluation_steps); + /** * Set this module's exported value for the name export_name to the specified * export_value. This method must be called only on Modules created via diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 446f1e2794f9..ce0017fb57be 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -2436,6 +2436,33 @@ Local Module::CreateSyntheticModule( i_module_name, i_export_names, evaluation_steps))); } +START_ALLOW_USE_DEPRECATED() +Local Module::CreateSyntheticModule( + Isolate* v8_isolate, Local module_name, + const MemorySpan>& export_names, + v8::Module::LegacySyntheticModuleEvaluationSteps evaluation_steps) { + // TODO(https://crbug.com/545375591): Remove once + // LegacySyntheticModuleEvaluationSteps is gone. +#if (__GNUC__ >= 8) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-function-type" +#endif + // Cast from 'v8::MaybeLocal (*)(v8::Local, + // v8::Local)' to 'v8::MaybeLocal + // (*)(v8::Local, v8::Local)'. Both return types are + // pointer-sized, trivially copyable handle wrappers, so they share the same + // representation. SyntheticModule::Evaluate() checks at runtime that the + // returned value really is a Promise. + auto promise_returning_steps = + reinterpret_cast(evaluation_steps); +#if (__GNUC__ >= 8) || defined(__clang__) +#pragma GCC diagnostic pop +#endif + return CreateSyntheticModule(v8_isolate, module_name, export_names, + promise_returning_steps); +} +END_ALLOW_USE_DEPRECATED() + Maybe Module::SetSyntheticModuleExport(Isolate* v8_isolate, Local export_name, Local export_value) { diff --git a/deps/v8/src/d8/d8.cc b/deps/v8/src/d8/d8.cc index 1c33ca83c369..67a43a3f653f 100644 --- a/deps/v8/src/d8/d8.cc +++ b/deps/v8/src/d8/d8.cc @@ -1441,7 +1441,7 @@ MaybeLocal Shell::FetchModuleTree(Local referrer, return result; } -MaybeLocal Shell::JSONModuleEvaluationSteps(Local context, +MaybeLocal Shell::JSONModuleEvaluationSteps(Local context, Local module) { Isolate* isolate = Isolate::GetCurrent(); diff --git a/deps/v8/src/d8/d8.h b/deps/v8/src/d8/d8.h index 8d57ddac8240..87c30015d211 100644 --- a/deps/v8/src/d8/d8.h +++ b/deps/v8/src/d8/d8.h @@ -884,9 +884,8 @@ class Shell : public i::AllStatic { const std::string& file_name, ModuleType module_type); - static MaybeLocal JSONModuleEvaluationSteps(Local context, - Local module); - + static MaybeLocal JSONModuleEvaluationSteps(Local context, + Local module); template static MaybeLocal CompileString(Isolate* isolate, Local context, Local source, diff --git a/deps/v8/src/objects/synthetic-module.cc b/deps/v8/src/objects/synthetic-module.cc index c80ce99fa73e..29d15516bfab 100644 --- a/deps/v8/src/objects/synthetic-module.cc +++ b/deps/v8/src/objects/synthetic-module.cc @@ -5,6 +5,7 @@ #include "src/objects/synthetic-module.h" #include "src/api/api-inl.h" +#include "src/base/macros.h" #include "src/builtins/accessors.h" #include "src/objects/js-generator-inl.h" #include "src/objects/module-inl.h" @@ -105,6 +106,15 @@ bool SyntheticModule::FinishInstantiate(Isolate* isolate, // Implements Synthetic Module Record's Evaluate concrete method: // https://heycam.github.io/webidl/#smr-evaluate +// The callback may have been created through the deprecated +// v8::Module::LegacySyntheticModuleEvaluationSteps overload, in which case it +// actually returns a v8::MaybeLocal and is called here through a +// mismatching signature. Both return types are pointer-sized, trivially +// copyable handle wrappers, so this is safe in practice, but it does trip +// CFI's and UBSan's indirect call checks. +// TODO(https://crbug.com/545375591): Remove DISABLE_CFI_ICALL once the +// deprecated overload is gone. +DISABLE_CFI_ICALL MaybeDirectHandle SyntheticModule::Evaluate( Isolate* isolate, DirectHandle module) { module->SetStatus(kEvaluating); @@ -112,6 +122,8 @@ MaybeDirectHandle SyntheticModule::Evaluate( v8::Module::SyntheticModuleEvaluationSteps evaluation_steps = FUNCTION_CAST( module->evaluation_steps()->foreign_address()); + // Deliberately received as a v8::Local: the deprecated callback + // signature only promises a Promise, it doesn't guarantee one. v8::Local result; if (!evaluation_steps(Utils::ToLocal(isolate->native_context()), Utils::ToLocal(Cast(module))) diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 910a18c61ff3..9d69054c9043 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -24701,37 +24701,48 @@ TEST(CodeCache) { isolate2->Dispose(); } -v8::MaybeLocal UnexpectedSyntheticModuleEvaluationStepsCallback( +v8::MaybeLocal UnexpectedSyntheticModuleEvaluationStepsCallback( Local context, Local module) { CHECK_WITH_MSG(false, "Unexpected call to synthetic module re callback"); } static int synthetic_module_callback_count; -v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( +v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( Local context, Local module) { synthetic_module_callback_count++; - // Synthetic module evaluation steps must return a Promise. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); return resolver->GetPromise(); } -v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackFail( +v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackFail( Local context, Local module) { synthetic_module_callback_count++; CcTest::isolate()->ThrowException( v8_str("SyntheticModuleEvaluationStepsCallbackFail exception")); - return v8::MaybeLocal(); + return v8::MaybeLocal(); } -v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackSetExport( +// Deprecated version of the evaluation steps, returning a MaybeLocal +// that holds a Promise. +// TODO(https://crbug.com/545375591): Remove together with +// v8::Module::LegacySyntheticModuleEvaluationSteps. +v8::MaybeLocal LegacySyntheticModuleEvaluationStepsCallback( + Local context, Local module) { + synthetic_module_callback_count++; + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); +} + +v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackSetExport( Local context, Local module) { Maybe set_export_result = module->SetSyntheticModuleExport( CcTest::isolate(), v8_str("test_export"), v8_num(42)); CHECK(set_export_result.FromJust()); - // Synthetic module evaluation steps must return a Promise. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); @@ -25074,6 +25085,40 @@ TEST(SyntheticModuleEvaluationStepsNoThrow) { CHECK_EQ(module->GetStatus(), Module::kEvaluated); } +// Covers the deprecated evaluation steps version, where the returned Promise is +// only checked at runtime. +// TODO(https://crbug.com/545375591): Remove together with +// v8::Module::LegacySyntheticModuleEvaluationSteps. +TEST(SyntheticModuleEvaluationStepsLegacyCallback) { + synthetic_module_callback_count = 0; + LocalContext env; + v8::Isolate* isolate = env.isolate(); + v8::Isolate::Scope iscope(isolate); + v8::HandleScope scope(isolate); + v8::Local context = v8::Context::New(isolate); + v8::Context::Scope cscope(context); + + auto export_names = std::to_array>({v8_str("default")}); + + START_ALLOW_USE_DEPRECATED() + Local module = v8::Module::CreateSyntheticModule( + isolate, + v8_str("SyntheticModuleEvaluationStepsLegacyCallback-" + "TestSyntheticModule"), + export_names, LegacySyntheticModuleEvaluationStepsCallback); + END_ALLOW_USE_DEPRECATED() + module->InstantiateModule(context, UnexpectedModuleResolveCallback) + .ToChecked(); + + CHECK_EQ(synthetic_module_callback_count, 0); + Local completion_value = module->Evaluate(context).ToLocalChecked(); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); + CHECK_EQ(synthetic_module_callback_count, 1); + CHECK_EQ(module->GetStatus(), Module::kEvaluated); +} + TEST(SyntheticModuleEvaluationStepsThrow) { synthetic_module_callback_count = 0; LocalContext env; @@ -27098,7 +27143,7 @@ MaybeLocal CheckResolveModuleWithImportSource( return v8::Module::CreateSyntheticModule( isolate, v8_str("my-mod"), {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Do nothing. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); diff --git a/deps/v8/test/unittests/objects/modules-unittest.cc b/deps/v8/test/unittests/objects/modules-unittest.cc index af910fd20f58..9777d8af353c 100644 --- a/deps/v8/test/unittests/objects/modules-unittest.cc +++ b/deps/v8/test/unittests/objects/modules-unittest.cc @@ -1686,7 +1686,7 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceName) { Local resource_name = NewString("synthetic-module"); Local module = Module::CreateSyntheticModule( isolate(), resource_name, {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Do nothing. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); @@ -1716,12 +1716,12 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceNameInError) { Local resource_name = NewString("synthetic-module"); Local module = Module::CreateSyntheticModule( isolate(), resource_name, {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Throw an error. Isolate* isolate = Isolate::GetCurrent(); isolate->ThrowException( v8::String::NewFromUtf8Literal(isolate, "synthetic module error")); - return MaybeLocal(); + return MaybeLocal(); }); CHECK_EQ(Module::kUninstantiated, module->GetStatus()); From 579df3c898577ebca9bc4069d7ce0d50edeefe6f Mon Sep 17 00:00:00 2001 From: Caio Lima Date: Mon, 17 Aug 2026 15:02:48 -0300 Subject: [PATCH 3/3] Change ModuleWrap::SyntheticModuleEvaluationStepsCallback to return a MaybeLocal --- src/module_wrap.cc | 8 ++++---- src/module_wrap.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 38ac15b337f3..8056f024b321 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -1442,7 +1442,7 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback( HostInitializeImportMetaObjectCallback); } -MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( +MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( Local context, Local module) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); @@ -1466,16 +1466,16 @@ MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( CHECK(!try_catch.Message().IsEmpty()); CHECK(!try_catch.Exception().IsEmpty()); try_catch.ReThrow(); - return MaybeLocal(); + return MaybeLocal(); } Local resolver; if (!Promise::Resolver::New(context).ToLocal(&resolver)) { - return MaybeLocal(); + return MaybeLocal(); } if (resolver->Resolve(context, Undefined(isolate)).IsNothing()) { - return MaybeLocal(); + return MaybeLocal(); } return resolver->GetPromise(); } diff --git a/src/module_wrap.h b/src/module_wrap.h index 14a8f1a4f2d6..f40976772cb9 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -185,7 +185,7 @@ class ModuleWrap : public BaseObject { const v8::FunctionCallbackInfo& args); static void SetInitializeImportMetaObjectCallback( const v8::FunctionCallbackInfo& args); - static v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( + static v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( v8::Local context, v8::Local module); static void SetSyntheticExport( const v8::FunctionCallbackInfo& args);