jimexist commented on code in PR #3529:
URL: https://github.com/apache/thrift/pull/3529#discussion_r3295747975
##########
lib/nodejs/lib/thrift/index.js:
##########
@@ -58,6 +58,34 @@ exports.createWebServer = web_server.createWebServer;
exports.Int64 = require("node-int64");
exports.Q = require("q");
+/**
+ * Convert a `node-int64` Int64 (as returned by `TBinaryProtocol.readI64`,
+ * `TCompactProtocol.readI64`, etc.) to a native `bigint`. Used by code
+ * generated with `js:bigint=true` to surface int64 values as BigInt without
+ * a protocol-layer toggle.
+ *
+ * @param {Int64} i64
+ * @returns {bigint}
+ */
+exports.toBigInt = function (i64) {
+ return i64.buffer.readBigInt64BE(i64.offset || 0);
+};
+
+/**
+ * Convert a native `bigint` to a `node-int64` Int64 suitable for passing to
+ * `writeI64`. Values outside the signed 64-bit range are wrapped to fit
+ * (`BigInt.asIntN(64, ...)`).
+ *
+ * @param {bigint} value
+ * @returns {Int64}
+ */
+exports.fromBigInt = function (value) {
+ const Int64 = exports.Int64;
+ const buf = Buffer.allocUnsafe(8);
+ buf.writeBigInt64BE(BigInt.asIntN(64, value), 0);
+ return new Int64(buf);
+};
Review Comment:
Good catch — fixed in 92e9f3efa. Added
`lib/nodejs/lib/thrift/bigint_compat.js` with feature-detected `readBigInt64BE`
/ `writeBigInt64BE` and a `readInt32BE` / `writeUInt32BE` + BigInt-shift
fallback for runtimes < Node 12 (and older browser `Buffer` polyfills).
`toBigInt` / `fromBigInt` now go through it. Tests cover the fallback path
explicitly (`bigint_helpers.test.js`, +16 assertions, total 48/48 pass).
##########
lib/nodejs/lib/thrift/browser.js:
##########
@@ -38,6 +38,33 @@ exports.createClient = require("./create_client");
exports.Int64 = require("node-int64");
exports.Q = require("q");
+/**
+ * Convert a `node-int64` Int64 to a native `bigint`. Used by code generated
+ * with `js:bigint=true`. Browser builds need `Buffer#readBigInt64BE`, which
+ * is shimmed by webpack's buffer polyfill / available natively in modern
+ * runtimes.
+ *
+ * @param {Int64} i64
+ * @returns {bigint}
+ */
+exports.toBigInt = function (i64) {
+ return i64.buffer.readBigInt64BE(i64.offset || 0);
+};
+
+/**
+ * Convert a native `bigint` to a `node-int64` Int64 for `writeI64`. Values
+ * outside the signed 64-bit range are wrapped (`BigInt.asIntN(64, ...)`).
+ *
+ * @param {bigint} value
+ * @returns {Int64}
+ */
+exports.fromBigInt = function (value) {
+ const Int64 = exports.Int64;
+ const buf = Buffer.allocUnsafe(8);
+ buf.writeBigInt64BE(BigInt.asIntN(64, value), 0);
+ return new Int64(buf);
+};
Review Comment:
Same fix as for the index.js thread — the browser entrypoint now also goes
through `bigint_compat`, which feature-detects `Buffer#readBigInt64BE` /
`writeBigInt64BE` and falls back to `readInt32BE` / `writeUInt32BE` + BigInt
arithmetic when the native methods are absent. That covers older Node runtimes
and older webpack `buffer` polyfills.
--
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]
