jimexist commented on code in PR #3529:
URL: https://github.com/apache/thrift/pull/3529#discussion_r3352685964


##########
compiler/cpp/src/thrift/generate/t_js_generator.cc:
##########
@@ -2587,7 +2643,14 @@ void t_js_generator::generate_serialize_field(ostream& 
out, t_field* tfield, str
         out << "writeI32(" << name << ")";
         break;
       case t_base_type::TYPE_I64:
-        out << "writeI64(" << name << ")";
+        // In bigint mode the generated field holds a `bigint`; convert
+        // back to a node-int64 Int64 before handing to `writeI64`, which
+        // expects either an Int64 or a Number.
+        if (gen_bigint_) {
+          out << "writeI64(thrift.fromBigInt(" << name << "))";
+        } else {
+          out << "writeI64(" << name << ")";
+        }

Review Comment:
   Good catch — fixed in 75e334f49. `thrift.fromBigInt` now coerces 
decimal-string and number inputs via `BigInt(value)` (the bigint branch is 
unchanged), so generated `map<i64, …>` write code that hands in `kiter` (a JS 
object-key string) works correctly. Also tightened the TS map type for bigint 
keys from `any` to `{ [k: string /*bigint*/]: V; }`, since JS coerces object 
keys to strings at runtime regardless. New tests in `bigint_helpers.test.js` 
cover the string/number coercion path including `MAX_I64` / `MIN_I64` decimal 
strings. 55/55 pass locally.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to