jimexist opened a new pull request, #3529:
URL: https://github.com/apache/thrift/pull/3529
## Summary
Adds two pieces of BigInt support to the `js:node` binding so int64 values
can flow through generated code and the binary protocol as native `bigint`
instead of `node-int64` `Int64` wrappers. Native `BigInt` has been available
since Node 10.4 and the published package already targets Node >= 10.18.0 via
`engines`, so every supported runtime can use it.
### Compiler — `js:bigint` flag (default `true`)
`compiler/cpp/src/thrift/generate/t_js_generator.cc` gains a `bigint` flag
that applies when `node` is in the option list. When enabled (the default):
- I64 constants are emitted as native BigInt literals (e.g. `42n`,
`9223372036854775807n`) instead of `new Int64(42)`.
- The `node-int64` import / require is dropped from `js_includes`,
`ts_includes`, and `ts_service_includes` (node + esm + ts variants).
- TypeScript I64 fields are declared as `bigint` in `.d.ts` instead of
`Int64`.
```sh
thrift --gen js:node MyService.thrift # bigint=true is the default
thrift --gen js:node,bigint=false MyService.thrift # legacy node-int64
output
```
The flag is silently forced `false` for plain `--gen js` (browser JS) so
callers that don't enable `node` are unaffected.
### Runtime — `TBinaryProtocol` `useBigInt` (opt-in)
`TBinaryProtocol` gains an optional fourth `options` argument with a
`useBigInt` flag. When set:
- `readI64` returns a native `bigint` instead of a `Thrift.Int64`.
- `writeI64` already accepts `bigint` regardless of the flag, so values can
be passed as `bigint`, `number`, or `Thrift.Int64`.
```js
const protocol = new thrift.TBinaryProtocol(transport, false, true, {
useBigInt: true });
```
The runtime default is unchanged in this PR. Users adopting the new compiler
default need to either:
1. Pass `useBigInt: true` to their `TBinaryProtocol` constructors, or
2. Opt out at the compiler with `bigint=false`.
Flipping the runtime default is a follow-up that requires rewriting the
integration test fixtures (`test_driver.mjs`, `test-cases.mjs`).
## Why
- Removes a per-int64 heap allocation and a `Buffer` wrapper.
- Lets users use arithmetic on int64 values directly (`a + b`) without
`.add()` / `.toNumber()` round-trips.
- Removes the runtime + d.ts coupling to `node-int64`, simplifying browser
bundling.
- The stale `lib/nodejs/README.md` claim of "node version 6 or later" is
fixed at the same time — it contradicted the `engines` field.
## Test coverage
- New `lib/nodejs/test/binary_protocol_bigint.test.js` — 25 assertions
covering `writeI64` accepting `bigint` / `number` / `Int64`; `readI64`
returning `Int64` by default; `readI64` returning `bigint` under `useBigInt:
true`; boundary values `I64_MAX` / `I64_MIN`; round-trips beyond
`Number.MAX_SAFE_INTEGER`. Passes locally.
- New `lib/nodejs/test/int64_bigint.test.js` — validates compiler-emitted
output against a `gen-nodejs-bigint/` fixture generated with the default
(bigint=true): typeof === "bigint", literal values, list elements, map shape,
absence of `require('node-int64')` and `new Int64(...)` in the generated
source. Will execute in CI once the compiler is rebuilt.
- All existing test generation invocations in `lib/nodejs/test/testAll.sh`
and `lib/nodets/test/testAll.sh` now pass `bigint=false` explicitly so
`int64.test.js`, `int64.test.ts`, `test_driver.mjs`, and `test-cases.mjs` keep
working with their existing `node-int64` semantics with zero rewriting.
## Files changed
- `compiler/cpp/src/thrift/generate/t_js_generator.cc` — flag parsing,
member, codegen for I64 const / TS type / imports.
- `lib/nodejs/lib/thrift/binary_protocol.js` — `useBigInt` plumbing on
`TBinaryProtocol`.
- `lib/nodejs/README.md` — corrected Node version, documented both layers of
BigInt mode.
- `lib/nodejs/test/testAll.sh` / `lib/nodets/test/testAll.sh` — pin existing
fixtures to `bigint=false`, add a separate bigint-default fixture for
`Int64Test.thrift`.
- `lib/nodejs/test/binary_protocol_bigint.test.js` (new),
`lib/nodejs/test/int64_bigint.test.js` (new).
## JIRA
No JIRA ticket filed yet — a committer should create one (suggested summary:
*"Add BigInt support to the nodejs runtime and js:node generator"*) and the PR
title / branch name can then be updated to match the `THRIFT-NNNN:` convention
per `AGENTS.md`.
## AI authorship
Per `AGENTS.md` § "AI-Generated Contributions", this change was drafted with
Claude Opus 4.7. The human author reviewed the diff and ran the runtime tests;
the compiler-side test relies on CI to rebuild the generator binary.
## Test plan
- [x] `node lib/nodejs/test/binary_protocol_bigint.test.js` — 25/25 pass
locally
- [ ] `lib/nodejs/test/testAll.sh` green in CI (full integration suite +
`int64.test.js` + `int64_bigint.test.js`)
- [ ] `lib/nodets/test/testAll.sh` green in CI
- [ ] Cross-language CI green (no wire-format change; only generator output
and an additive runtime option)
--
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]
