[
https://issues.apache.org/jira/browse/THRIFT-6322?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6322.
--------------------------------
Fix Version/s: 0.26.0
Assignee: Jens Geyer
Resolution: Fixed
> lib/js: the browser tests call struct read and write as plain properties
> ------------------------------------------------------------------------
>
> Key: THRIFT-6322
> URL: https://issues.apache.org/jira/browse/THRIFT-6322
> Project: Thrift
> Issue Type: Bug
> Components: JavaScript - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.26.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{lib/js/test/deep-constructor.test.js}} fails four of its assertions:
> {code}
> >> Serialize/deserialize simple struct should return equal object
> >> Message: Died on test #1 ... data.write is not a function
> >> TypeError: data.write is not a function
> >> at serialize (deep-constructor.test.js:24)
> 5 tests completed with 4 failed, 0 skipped, and 0 todo.
> 72 assertions, passed: 68, failed: 4
> {code}
> The JavaScript generator keys the struct methods by symbol:
> {code}
> Simple.prototype[Symbol.for("read")] = function(input) { ... };
> Simple.prototype[Symbol.for("write")] = function(output) { ... };
> {code}
> and generated code calls them the same way, for example in
> {{gen-js/ThriftTest_types.js}}:
> {code}
> this.struct_thing[Symbol.for("read")](input);
> this.struct_thing[Symbol.for("write")](output);
> {code}
> The tests still use plain property access. Every site, from a sweep of the
> browser test
> code for both spellings:
> || File || Line || Call ||
> | {{lib/js/test/deep-constructor.test.js}} | 24 | {{data.write(protocol)}} |
> | {{lib/js/test/deep-constructor.test.js}} | 35 | {{data.read(protocol)}} |
> | {{lib/js/test/test.js}} | 244 | {{xtruct3.read(input)}} |
> | {{lib/js/test/phantom-client.js}} | 268 | {{xtruct3.read(input)}} |
> Nothing else in the library or in the generated code does: those all go
> through
> {{Symbol.for(...)}}.
> h2. Why nobody noticed
> The test has not run. {{lib/js/Makefile.am}} guards {{check-local}} with {{if
> HAVE_NPM}}, and
> {{configure.ac}} looks for npm only inside the {{--with-nodejs}} and
> {{--with-nodets}} blocks.
> Any build that disables the nodejs binding leaves {{NPM}} empty, so {{make -C
> lib/js check}}
> reports "Nothing to be done" and exits 0. THRIFT-6319 corrects that, which is
> what brought this
> failure to light.
> h2. Verified
> Reproduced on master {{82bab6306}}, and with the generator changes of
> {{THRIFT-6317}} and
> {{THRIFT-6318}} reverted -- the failure is identical either way, so it does
> not come from those.
> Getting this far also needs the jshint language levels corrected, otherwise
> {{jshint:test}}
> aborts the run before the tests.
> Switching the two sites in {{deep-constructor.test.js}} to
> {{Symbol.for(...)}} turns that page
> green -- 5 tests, 75 assertions, 0 failed -- and the run then advances to
> {{test-nojq.html}},
> which fails on the {{test.js}} site with the same message. So the list above
> is the whole fix.
> Drafted with AI assistance (Claude Opus 5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
