Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4406

JavaScript: Use modern Promise implementations

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0
    • Component/s: JavaScript - Compiler
    • Labels:
      None

      Description

      I would like to acces the results of service method invocations in a web browser environment using Promises (or a compatible API). I'm willing to work on the necessary changes in the compiler and am seeking guidance regarding the best way to make these changes.

      Current state:

      In browser JS, i can pass a callback function as last argument when calling a method:

          service.getFoo('a1', 'a2', function(foo) {
              alert(foo);
          });
      

      There is also a synchronous API, however this blocking way is discouraged by browser vendors. It also fails with an error for me when using WebSocketTransport instead of XHRTransport.

          alert(service.getFoo('a1', 'a2'));
      

      In JS code generated for NodeJS, the 'q' library is included, which provides an API mostly compatible with JavaScript's native Promises.

          service.getFoo('a1', 'a2').then(function(foo) {
              alert(foo);
          });
      

      From what I can tell 'q' is mostly irrelevant today since native Promises are supported in NodeJs for many versions.

      Regarding support in browsers, native Promises work in recent versions of most browsers and can be enabled in other cases by including one of the many polyfill libraries, like the popular core-js.

      Proposed solution:

      In both browser and nodeJS, service methods return a Promise when invoked without a callback. This has many advantages for developers:

      • Promise API is an established standard in modern JavaScript (ECMAScript), it is easy for developers to consume and to use with other libraries
      • Error handling is possible through catch()
      • Result handling is very flexible through chaining
      • Promises can be combined through Promise.all() and Promise.race()
      • The await keyword makes it even easier to write async code. (Support limited to some browsers or with cross-compilers like babel)

      However, this would break compatibility:

      • Synchronous (blocking) invocations would no longer be supported, instead a Promise instead of the value is returned
      • with NodeJS code relying on some features specific to 'q' (e.g. finally() and {{done()`)
      • Internet Explorer needs a polyfill for Promises

      I'm assuming the code generated by Thrift should stay backwards compatible in the foreseeable future, so I suggest making these changes behind a new compiler flag, -gen js:es6 (for ECMAScript 6). This would also open up the ability to leverage more ES6 features in the future: For example, using const instead of var might give a slight performance improvement and using the class syntax could improve readability of the generated code.

      Please share any thoughts regarding this topic or hints towards a good implementation. If there are no objections, I'm willing to start working on this soon, with the hopes of getting my changes accepted into the project codebase.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jking3 James E. King III
                Reporter:
                bananer Philip Frank
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: