THRIFT-6040: Switch JS/Node generator and runtime from Q to native Promise

compiler/cpp/src/thrift/generate/t_js_generator.cc

@@ -70,6 +70,7 @@ class t_js_generator : public t_oop_generator {
     gen_es6_ = false;
     gen_esm_ = false;
     gen_episode_file_ = false;
+    gen_native_promise_ = true;
 
     bool with_ns_ = false;
 
@@ -90,6 +91,12 @@ class t_js_generator : public t_oop_generator {
         parse_imports(program, iter->second);
       } else if (iter->first.compare("thrift_package_output_directory") == 0) {
         parse_thrift_package_output_directory(iter->second);
+      } else if (iter->first.compare("native_promise") == 0) {
+        if (iter->second == "false" || iter->second == "0" || iter->second == "no") {
+          gen_native_promise_ = false;
+        } else {
+          gen_native_promise_ = true;
+        }
       } else {
         throw std::invalid_argument("unknown option js:" + iter->first);
       }
@@ -388,6 +395,12 @@ class t_js_generator : public t_oop_generator {
    */
   bool gen_episode_file_;
 
+  /**
+   * True (default) if generated code should use native Promise; false to emit
+   * the legacy Q-based output (Q.fcall / Q.defer and imports from the 'q' package).
+   */
+  bool gen_native_promise_;
+
   /**
    * The name of the defined module(s), for TypeScript Definition Files.
    */
@@ -530,11 +543,11 @@ string t_js_generator::js_includes() {
       result += js_const_type_ + "thrift = require('thrift');\n"
           + js_const_type_ + "Thrift = thrift.Thrift;\n";
     }
-    if (!gen_es6_) {
+    if (!gen_native_promise_ && !gen_es6_) {
       if (gen_esm_) {
-        result += "import { Q } from 'thrift';\n";
+        result += "import Q from 'q';\n";
       } else {
-        result += js_const_type_ + "Q = thrift.Q;\n";
+        result += js_const_type_ + "Q = require('q');\n";
       }
     }
     if (gen_esm_) {
@@ -556,13 +569,17 @@ string t_js_generator::js_includes() {
  */
 string t_js_generator::ts_includes() {
   if (gen_node_) {
-    return string(
+    string result =
         "import thrift = require('thrift');\n"
-        "import Thrift = thrift.Thrift;\n"
-        "import Q = thrift.Q;\n"
+        "import Thrift = thrift.Thrift;\n";
+    if (!gen_native_promise_) {
+      result += "import Q = require('q');\n";
+    }
+    result +=
         "import Int64 = require('node-int64');\n"
         "import { v4 as uuid } from 'uuid';\n"
-        "type uuid = string;");
+        "type uuid = string;";
+    return result;
   }
   return string(
     "import Int64 = require('node-int64');\n"
@@ -575,11 +592,14 @@ string t_js_generator::ts_includes() {
  */
 string t_js_generator::ts_service_includes() {
   if (gen_node_) {
-    return string(
+    string result =
         "import thrift = require('thrift');\n"
-        "import Thrift = thrift.Thrift;\n"
-        "import Q = thrift.Q;\n"
-        "import Int64 = require('node-int64');");
+        "import Thrift = thrift.Thrift;\n";
+    if (!gen_native_promise_) {
+      result += "import Q = require('q');\n";
+    }
+    result += "import Int64 = require('node-int64');";
+    return result;
   }
   return string("import Int64 = require('node-int64');");
 }
@@ -1572,6 +1592,10 @@ void t_js_generator::generate_process_function(t_service* tservice, t_function*
 
   if (gen_es6_) {
     indent(f_service_) << "new Promise((resolve) => resolve(this._handler." << tfunction->get_name() << ".bind(this._handler)(" << '\n';
+  } else if (gen_native_promise_) {
+    // Non-ES6 native Promise: use function expression with explicit `this`
+    // binding so we don't rely on arrow-function lexical `this`.
+    indent(f_service_) << "new Promise(function(resolve) { resolve(this._handler." << tfunction->get_name() << ".bind(this._handler)(" << '\n';
   } else {
     string maybeComma = (fields.size() > 0 ? "," : "");
     indent(f_service_) << "Q.fcall(this._handler." << tfunction->get_name() << ".bind(this._handler)"
@@ -1587,6 +1611,8 @@ void t_js_generator::generate_process_function(t_service* tservice, t_function*
 
   if (gen_es6_) {
     indent(f_service_) << "))).then(result => {" << '\n';
+  } else if (gen_native_promise_) {
+    indent(f_service_) << ")); }.bind(this)).then(function(result) {" << '\n';
   } else {
     indent(f_service_) << ").then(function(result) {" << '\n';
   }
@@ -1951,22 +1977,44 @@ void t_js_generator::generate_service_client(t_service* tservice) {
       f_service_ << indent() << "this._seqid = this.new_seqid();" << '\n' << indent()
                  << "if (callback === undefined) {" << '\n';
       indent_up();
-      f_service_ << indent() << js_const_type_ << "_defer = Q.defer();" << '\n' << indent()
-                 << "this._reqs[this.seqid()] = function(error, result) {" << '\n';
-      indent_up();
-      indent(f_service_) << "if (error) {" << '\n';
-      indent_up();
-      indent(f_service_) << "_defer.reject(error);" << '\n';
-      indent_down();
-      indent(f_service_) << "} else {" << '\n';
-      indent_up();
-      indent(f_service_) << "_defer.resolve(result);" << '\n';
-      indent_down();
-      indent(f_service_) << "}" << '\n';
-      indent_down();
-      indent(f_service_) << "};" << '\n';
-      f_service_ << indent() << "this.send_" << funname << "(" << arglist << ");" << '\n'
-                 << indent() << "return _defer.promise;" << '\n';
+      if (gen_native_promise_) {
+        f_service_ << indent() << js_const_type_ << "self = this;" << '\n' << indent()
+                   << "return new Promise(function(resolve, reject) {" << '\n';
+        indent_up();
+        f_service_ << indent() << "self._reqs[self.seqid()] = function(error, result) {" << '\n';
+        indent_up();
+        indent(f_service_) << "if (error) {" << '\n';
+        indent_up();
+        indent(f_service_) << "reject(error);" << '\n';
+        indent_down();
+        indent(f_service_) << "} else {" << '\n';
+        indent_up();
+        indent(f_service_) << "resolve(result);" << '\n';
+        indent_down();
+        indent(f_service_) << "}" << '\n';
+        indent_down();
+        indent(f_service_) << "};" << '\n';
+        f_service_ << indent() << "self.send_" << funname << "(" << arglist << ");" << '\n';
+        indent_down();
+        indent(f_service_) << "});" << '\n';
+      } else {
+        f_service_ << indent() << js_const_type_ << "_defer = Q.defer();" << '\n' << indent()
+                   << "this._reqs[this.seqid()] = function(error, result) {" << '\n';
+        indent_up();
+        indent(f_service_) << "if (error) {" << '\n';
+        indent_up();
+        indent(f_service_) << "_defer.reject(error);" << '\n';
+        indent_down();
+        indent(f_service_) << "} else {" << '\n';
+        indent_up();
+        indent(f_service_) << "_defer.resolve(result);" << '\n';
+        indent_down();
+        indent(f_service_) << "}" << '\n';
+        indent_down();
+        indent(f_service_) << "};" << '\n';
+        f_service_ << indent() << "this.send_" << funname << "(" << arglist << ");" << '\n'
+                   << indent() << "return _defer.promise;" << '\n';
+      }
       indent_down();
       indent(f_service_) << "} else {" << '\n';
       indent_up();
@@ -3132,6 +3180,9 @@ THRIFT_REGISTER_GENERATOR(js,
                           "    ts:              Generate TypeScript definition files.\n"
                           "    with_ns:         Create global namespace objects when using node.js\n"
                           "    es6:             Create ES6 code with Promises\n"
+                          "    native_promise=[true|false]:\n"
+                          "                     Use native Promise (default true). Set to false to\n"
+                          "                     emit legacy Q-based output (requires the 'q' package).\n"
                           "    thrift_package_output_directory=<path>:\n"
                           "                     Generate episode file and use the <path> as prefix\n"
                           "    imports=<paths_to_modules>:\n"

lib/nodejs/lib/thrift/browser.js

@@ -36,7 +36,6 @@ exports.createOhosClient = ohosConnection.createOhosClient;
 exports.createClient = require("./create_client");
 
 exports.Int64 = require("node-int64");
-exports.Q = require("q");
 
 var mpxProtocol = require("./multiplexed_protocol");
 exports.Multiplexer = mpxProtocol.Multiplexer;

lib/nodejs/lib/thrift/index.js

@@ -56,7 +56,6 @@ var web_server = require("./web_server");
 exports.createWebServer = web_server.createWebServer;
 
 exports.Int64 = require("node-int64");
-exports.Q = require("q");
 
 var mpxProcessor = require("./multiplexed_processor");
 var mpxProtocol = require("./multiplexed_protocol");

lib/nodets/test/test_driver.ts

@@ -30,7 +30,6 @@ import test = require("tape");
 import ttypes = require("./gen-nodejs/ThriftTest_types");
 import ThriftTest = require("./gen-nodejs/ThriftTest");
 import thrift = require("thrift");
-import Q = thrift.Q;
 import TException = thrift.Thrift.TException;
 var Int64 = require("node-int64");
 import testCases = require("./test-cases");
@@ -129,11 +128,11 @@ export function ThriftTestDriverPromise(
   client: ThriftTest.Client,
   callback: (status: string) => void,
 ) {
-  test("Q Promise Client Tests", function (assert) {
+  test("Promise Client Tests", function (assert) {
     var checkRecursively = makeRecursiveCheck(assert);
 
     function fail(msg: string) {
-      return function (error, response) {
+      return function (error) {
         if (error !== null) {
           assert.fail(msg);
         }
@@ -149,7 +148,7 @@ export function ThriftTestDriverPromise(
           .then(function (actual: any) {
             assertionFn(actual, expected, fnName);
           })
-          .fail(fail("fnName"));
+          .catch(fail(fnName));
       };
     }
 
@@ -161,58 +160,58 @@ export function ThriftTestDriverPromise(
     );
     testCases.deep.forEach(makeAsserter(assert.deepEqual));
 
-    Q.resolve(client.testStruct(testCases.out))
+    Promise.resolve(client.testStruct(testCases.out))
       .then(function (response) {
         checkRecursively(testCases.out, response, "testStruct");
       })
-      .fail(fail("testStruct"));
+      .catch(fail("testStruct"));
 
-    Q.resolve(client.testNest(testCases.out2))
+    Promise.resolve(client.testNest(testCases.out2))
       .then(function (response) {
         checkRecursively(testCases.out2, response, "testNest");
       })
-      .fail(fail("testNest"));
+      .catch(fail("testNest"));
 
-    Q.resolve(client.testInsanity(testCases.crazy))
+    Promise.resolve(client.testInsanity(testCases.crazy))
       .then(function (response) {
         checkRecursively(testCases.insanity, response, "testInsanity");
       })
-      .fail(fail("testInsanity"));
+      .catch(fail("testInsanity"));
 
-    Q.resolve(client.testException("TException"))
+    Promise.resolve(client.testException("TException"))
       .then(function (response) {
-        fail("testException: TException");
+        assert.fail("testException: TException");
       })
-      .fail(function (err) {
+      .catch(function (err) {
         assert.ok(err instanceof TException);
       });
 
-    Q.resolve(client.testException("Xception"))
+    Promise.resolve(client.testException("Xception"))
       .then(function (response) {
-        fail("testException: Xception");
+        assert.fail("testException: Xception");
       })
-      .fail(function (err) {
+      .catch(function (err) {
         assert.ok(err instanceof ttypes.Xception);
         assert.equal(err.errorCode, 1001);
         assert.equal("Xception", err.message);
       });
 
-    Q.resolve(client.testException("no Exception"))
+    Promise.resolve(client.testException("no Exception"))
       .then(function (response) {
         assert.equal(undefined, response); //void
       })
-      .fail(fail("testException"));
+      .catch(fail("testException"));
 
     client.testOneway(0, fail("testOneway: should not answer"));
 
     checkOffByOne(function (done) {
-      Q.resolve(client.testI32(-1))
+      Promise.resolve(client.testI32(-1))
         .then(function (response) {
           assert.equal(-1, response);
           assert.end();
           done();
         })
-        .fail(fail("checkOffByOne"));
+        .catch(fail("checkOffByOne"));
     }, callback);
   });
 }