[
https://issues.apache.org/jira/browse/THRIFT-2350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13895025#comment-13895025
]
Randy Abernethy commented on THRIFT-2350:
-----------------------------------------
Hi Henrique,
Thanks for the commit!
In regards to keeping Node.js source out of the lib/js/test directory, I think
I disagree. Presently the JS Java test server is housed there
lib/js/test/src/test/Httpd.java). Its role is not to support Java but to
support JavaScript, so the location makes some sense. That said I would rather
see this Java server moved to the integration test suit because it is really a
cross language test not a JavaScript test.
In the context of Apache Thrift, Browser based JavaScript is half a language,
because you can only build clients a server language is needed. If the choice
is between Java and Node.js, I think we get many benefits by using Node.js.
Node.js is the defacto server side JavaScript language, uses much of the same
Apache Thrift compiler code and has many library commonalities. Almost all of
the build and project management tools for front end JS are also written in
Node.js so it will likely already be in place for browser devs.
With the server_http.js and test_handler.js in lib/js/test you can go to the
lib/js directory, type "npm install" and then "grunt" and the system will lint,
concat, min, gen docs and test the JS source using phantom against the Node.js
server_http.js. I think this will be straight forward for JS developers, unlike
building a Java server. Grunt is the natural counter part to ant/maven/make
used elsewhere in the thrift build. At some point I think make should simply
drive grunt in the lib/js tree much as it does with ant in lib/java. With the
new compiler-only switch I can clone the master, make just the compiler and
grunt lib/js and I am ready to work with JavaScript.
Another thought is that the server_http.js may grow to be more front end test
focused over time (Web Sockets, etc), making it less of a fit on the node test
side. Also if we include the server_http.js and test_handler.js in lib/js/test
and we point Bower at the lib/js path (as Roger suggested) rather than
lib/js/src or lib/js/dist, all you need to do to pull a full JS environment
into your project is "bower install thrift". You can then "npm install" and
"grunt" to build the project (if you have a thrift compiler) with a complete
running client/server test, which is very handy as a tutorial. These files are
all small.
I would also say that leaving server_http.js in lib/nodejs/test is fine but if
so we need to create a nodejs client to actually test with it. It has no real
purpose in the nodejs tree right now and is not wired into testAll.sh or any
other test.
If there is support for creating the described Node JavaScript server in the
JavaScript lib/js/test dir, I can add a patch that creates (or moves)
lib/js/test/server_http.js and lib/js/test/test_handler.js. If not I need to
add a patch to fix the grunt build, it is broken now because of the
lib/js/test/server_http.js dependency.
Interested to hear other views!
Best,
Randy
> Add async calls to normal JavaScript
> ------------------------------------
>
> Key: THRIFT-2350
> URL: https://issues.apache.org/jira/browse/THRIFT-2350
> Project: Thrift
> Issue Type: Improvement
> Components: JavaScript - Compiler, JavaScript - Library
> Affects Versions: 0.9.2
> Environment: All
> Reporter: Randy Abernethy
> Assignee: Henrique Mendonça
> Priority: Minor
> Fix For: 0.9.2
>
> Attachments:
> 0001-add-async-support-to-default-JavaScript-client.patch,
> 0002-updated-grunfile-and-tests-for-async.patch,
> 0003-reorg-of-js-tests-removed-js-tests-from-node-dir.patch
>
>
> Currently async call are only available with -gen js if you unwire the client
> send/recv methods (pretty messy and unintuitive) or if you use jQuery (-gen
> js:jquery). This patch makes it easy to use async callbacks with any Thrift
> call by simply appending the desired callback to the args). e.g.
> client.myFunc(arg1, arg2, function (result) {
> //do callback stuff with result
> //result will be the normal return value or the exception
> });
> This patch preserves the existing sync call style (just leave off the
> callback).
> Compiler js generator changes
> =============================
> - Combining node and jquery switches now results in a compile error rather
> than silently producing corrupt code in ./gen-nodejs
> - Updated comments
> - Added ability to make async calls without jQuery
> Node test client.js
> ===================
> - Corrected comments and spelling
> JS thrift.js
> ====================
> - Added support for async calls w/o jQuery
> Node/JS test.js
> ====================
> - Updated comments to clarify jQuery dependency (-gen js:jquery)
> Node/JS test-nojq.js test-nojq.html
> ===================================
> - Full test suite for normal JS build (-gen js)
> Node http test server and handler
> =================================
> linted
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
