[
https://issues.apache.org/jira/browse/THRIFT-3122?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14541220#comment-14541220
]
ASF GitHub Bot commented on THRIFT-3122:
----------------------------------------
Github user itkach commented on the pull request:
https://github.com/apache/thrift/pull/476#issuecomment-101486364
> Tests for the browser side would be awesome
Existing browser tests pass when executed locally (make check with Java
server is successful and grunt test fails only two tests involving the very
long string because my node, or rather iojs, server pukes for some reason -
doesn't appear to be related to my changes), in addition to unit tests running
under node. If I understand correctly browser tests run in PhantomJS, which is
basically some version of WebKit, which is ECMA5 js environment - not that the
new code assumes ECMA5, but if concern is about js versions compatibility then
"browser" tests don't really give a lot of extra assurances compared to tests
under node. So I'm not quite sure what additional tests are you looking for.
Can you elaborate?
> and, as I said before, putting the copy functions in the libs instead of
copying them on every service would be nice.
When you say `in the libs` do you actually mean two places -
`nodejs/lib/thrift/` and `js/src/thrift.js`? Just to clarify - you would like
the new list/map copy functions to be added in (copied into) these two places
instead of a single place in `t_js_generator.cc`?
> Javascript struct constructor should properly initialize struct and container
> members from plain js arguments
> -------------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-3122
> URL: https://issues.apache.org/jira/browse/THRIFT-3122
> Project: Thrift
> Issue Type: Improvement
> Components: JavaScript - Compiler
> Reporter: Igor Tkach
>
> Currently constructors for struct types in generated javascript accept
> {{args}} object and initialize struct's members by simply assigning a value
> from corresponding {{args}} object property (if not undefined). If struct
> member is
> another struct it must be explicitly created with constructor and passed as
> an argument value.
> Given following definitions:
> {code}
> struct A {
> 1: string something
> }
> struct B {
> 1: A value
> }
> {code}
> this works:
> {code:javascript}
> var b1 = new B(
> {
> value: new A(
> {
> something: 'hello'
> }
> )
> }
> );
> {code}
> this doesn't:
> {code:javascript}
> var b2 = new B(
> {
> value: {
> something: 'hello'
> }
> }
> );
> {code}
> Attempt to serialize b2 will result in error because {{b2.a}} doesn't have a
> {{write}} method.
> This becomes especially problematic when deep objects are used with libraries
> like [Underscore.js|http://underscorejs.org/], [lodash|https://lodash.com/],
> [React's immutability
> helpers|https://facebook.github.io/react/docs/update.html] or
> [Immutable.js|https://github.com/facebook/immutable-js]: most operations will
> return or produce plain javascript objects without read/write methods even if
> Thrift objects were given as input. Manually converting object graphs back to
> Thrift serializable form is not workable.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
