Dynamic import should return different promise objects

Author: rhuanjl

lib/Runtime/Language/SourceTextModuleRecord.cpp

@@ -202,7 +202,7 @@ namespace Js
 
             if (this->promise != nullptr)
             {
-                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this);
+                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this, false);
             }
 
             // Notify host if current module is dynamically-loaded module, or is root module and the host hasn't been notified
@@ -307,7 +307,7 @@ namespace Js
                 {
                     // Cleanup in case of error.
                     this->ReleaseParserResources();
-                    SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, scriptContext, this);
+                    SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, scriptContext, this, false);
                 }
                 else
                 {
@@ -342,7 +342,7 @@ namespace Js
             }
         }
 
-        return this->promise;
+        return JavascriptPromise::CreatePassThroughPromise(this->promise, scriptContext);
     }
 
     HRESULT SourceTextModuleRecord::PrepareForModuleDeclarationInitialization()
@@ -401,7 +401,7 @@ namespace Js
 
             if (this->promise != nullptr)
             {
-                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this);
+                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this, false);
             }
 
             if (this->promise != nullptr || (isRootModule && !hadNotifyHostReady))
@@ -967,7 +967,7 @@ namespace Js
 
             if (this->promise != nullptr)
             {
-                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this);
+                SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this, false);
                 return scriptContext->GetLibrary()->GetUndefined();
             }
             else
@@ -1029,7 +1029,7 @@ namespace Js
             this->errorObject = errorObject;
             if (this->promise != nullptr)
             {
-                ResolveOrRejectDynamicImportPromise(false, errorObject, scriptContext, this);
+                ResolveOrRejectDynamicImportPromise(false, errorObject, scriptContext, this, false);
                 return scriptContext->GetLibrary()->GetUndefined();
             }
         }
@@ -1041,7 +1041,7 @@ namespace Js
 
         if (this->promise != nullptr)
         {
-            SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(true, this->GetNamespace(), this->GetScriptContext(), this);
+            SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(true, this->GetNamespace(), this->GetScriptContext(), this, false);
         }
 
         return ret;
@@ -1268,7 +1268,7 @@ namespace Js
     }
 
     // static
-    Var SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(bool isResolve, Var value, ScriptContext *scriptContext, SourceTextModuleRecord *moduleRecord)
+    Var SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(bool isResolve, Var value, ScriptContext *scriptContext, SourceTextModuleRecord *moduleRecord, bool useReturn)
     {
         bool isScriptActive = scriptContext->GetThreadContext()->IsScriptActive();
         JavascriptPromise *promise = nullptr;
@@ -1297,8 +1297,11 @@ namespace Js
         if (moduleRecord != nullptr)
         {
             moduleRecord->SetPromise(nullptr);
+            if (useReturn)
+            {
+                return JavascriptPromise::CreatePassThroughPromise(promise, scriptContext);
+            }
         }
-
         return promise;
     }
 }

lib/Runtime/Language/SourceTextModuleRecord.h

@@ -108,7 +108,7 @@ namespace Js
 
         void SetParent(SourceTextModuleRecord* parentRecord, LPCOLESTR moduleName);
         Utf8SourceInfo* GetSourceInfo() { return this->pSourceInfo; }
-        static Var ResolveOrRejectDynamicImportPromise(bool isResolve, Var value, ScriptContext *scriptContext, SourceTextModuleRecord *mr = nullptr);
+        static Var ResolveOrRejectDynamicImportPromise(bool isResolve, Var value, ScriptContext *scriptContext, SourceTextModuleRecord *mr = nullptr, bool useReturn = true);
         Var PostProcessDynamicModuleImport();
 
     private:

test/es6module/dynamic-module-functionality.js

@@ -406,6 +406,26 @@ var tests = [
         body: function() {
             assert.throws(()=>{eval('new import("ModuleSimpleExport.js")')}, SyntaxError);
         }
+    },
+    {
+        name : "Test that import() always gives different promise objects - Bug Issue #5795",
+        body: function () {
+            WScript.RegisterModuleSource("testModule", "export const a = 5;");
+            let functionBody =
+                `testDynamicImport(function () {
+                    const first = import ('ModuleSimpleExport.js');
+                    const second = import ('ModuleSimpleExport.js');
+                    assert.isTrue(first !== second, 'import() should not return the same promise');
+                    return Promise.all([first, second]).then ((results) => ({first, second, results}));
+                }, function (imports) {
+                    assert.isTrue(imports.first !== imports.second, 'import() should not return the same promise after resolution');
+                    assert.isTrue(imports.results[0] === imports.results[1], 'import() should return the same namespace object');
+                }, function (e) {
+                    print ("Test should not throw, threw " + e);
+                }, _asyncEnter, _asyncExit);`;
+            testScript(functionBody, "Test that import() always gives different promise objects", false, true);
+            testModuleScript(functionBody, "Test that import() always gives different promise objects", false, true);
+        }
     }
 ];