feat: add support for Node wrapper with TypeScript and dual-publishing (ESM/CJS)

[surbhigarg92]

This PR introduces a Node.js wrapper (spannerlib-node) for the Spanner shared library. It uses TypeScript and Node-API to provide a dual-published package (ESM and CommonJS) .

Key Highlights

1. Dual Publishing: Successfully ships targeted builds inside build/esm/ and build/cjs/ mapped through an exports map.
2. executeSql Support: Implemented the core method to execute SQL statements against the database in Node wrapper. Transactions, sessions, and other administrative methods will be built out in a follow-up PR.
3. Raw Protobuf Rows: rows.next() returns raw ListValue objects, matching Java and .NET behaviors.
4. Automated Native Build: Added a custom shell script to handle Go binary compilation on the fly, chained directly into the standard npm run build lifecycle.

Current Limitations
Windows CI: The Windows CI currently fails on compile-time static linking. We have deferred Windows support and platform-specific packaging to a dedicated task after this merges.

[gemini-code-assist]

Code Review

This pull request introduces a new Node.js wrapper for the Spanner shared library, providing three implementation approaches (N-API, Koffi, and IPC) along with a benchmarking suite. The review identified several critical issues, including hardcoded paths in build scripts, synchronous execution of potentially blocking operations, missing memory management for pinned objects, and incomplete type handling in the result parser. I recommend addressing these portability and performance concerns before merging.

I am having trouble creating individual review comments. Click here to see my feedback.

spannerlib/grpc-server/build-protos.sh (31)

Hardcoded absolute path to the Java gRPC plugin. This will fail on any machine other than the author's. Consider using an environment variable or ensuring the plugin is available in the system PATH.

spannerlib/grpc-server/build-protos.sh (40)

Hardcoded absolute path to the C# gRPC plugin. This breaks the portability of the build script and will cause CI/CD pipelines to fail.

spannerlib/wrappers/spannerlib-node/src/cpp/addon.cc (303-321)

CloseRowsWrapper is implemented synchronously. Closing rows in a database driver often involves network communication to signal the end of a stream or release server-side resources. Calling this on the Node.js main thread can block the event loop, leading to performance degradation. It should be implemented using Napi::AsyncWorker.

spannerlib/wrappers/spannerlib-node/src/ffi/utils.ts (15-43)

The invokeAsync function accepts constructor1 and constructor2 (which should be renamed to refInstance and pinManager) but never uses them to register the pinner ID (result.r0) with the memory manager. This will lead to memory leaks in the Go shared library because pinned objects are never released when JavaScript objects are garbage collected.

spannerlib/wrappers/spannerlib-node/src/lib/connection.ts (43)

The session ID is hardcoded to "poc/dummy". Cloud Spanner requires a valid session ID for executing SQL. This will cause errors when interacting with a real Spanner instance. The session should be dynamically obtained from the pool.

spannerlib/wrappers/spannerlib-node/src/lib/rows.ts (24-39)

The parseRowToObject function only handles a subset of Spanner types. It is missing support for bytesValue, structValue, and listValue. This could lead to data loss or undefined values being returned without warning for those types.

spannerlib/wrappers/spannerlib-node/src/lib/rows.ts (72)

Fetching rows one by one (numRows: 1) is highly inefficient as it incurs a network round-trip for every single row. Consider using a larger batch size or making it configurable to improve throughput.

spannerlib/wrappers/spannerlib-nodejs-poc/benchmark/benchmark.js (31)

The database path is hardcoded to a specific testing instance. This should be configurable via environment variables to allow running benchmarks in different environments.

const dbPath = process.env.SPANNER_DB_PATH || `projects/span-cloud-testing/instances/gargsurbhi-testing/databases/${dbName}`;

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a Node-API wrapper for the Spanner shared library, providing an object-oriented TypeScript interface for managing database pools, connections, and row iterators. The implementation includes a C++ bridge and a memory management system using a FinalizationRegistry. However, several critical issues were identified regarding memory management and performance. Specifically, memory leaks occur in Rows.next() and CloseRowsWrapper because native pinner IDs are not released, and the Pool, Connection, and Rows classes are not correctly registered with the cleanup registry. Additionally, CloseRowsWrapper should be implemented asynchronously to avoid blocking the Node.js event loop, and hardcoded session values in SQL execution must be addressed for production use.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a Node-API wrapper for the Spanner shared library, featuring a C++ addon to bridge Node.js and Go, along with TypeScript classes for managing pools, connections, and row iteration. The implementation includes automatic memory management via a FinalizationRegistry and comprehensive build scripts. Feedback highlights a critical race condition in the C++ addon where memory might be released before being copied, as well as potential issues with promises hanging due to insufficient argument validation. Other suggestions include improving type safety in the TypeScript layer and addressing hardcoded session identifiers.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a Node-API wrapper for the Spanner shared library, providing a high-performance bridge between Node.js and Go. It includes a C++ addon for handling native calls, TypeScript classes for managing pools, connections, and row iteration, and a memory management system using a FinalizationRegistry. The review identifies several critical memory management issues, specifically regarding the failure to release pinner IDs in error paths and after data copying in the C++ layer. Additionally, the FinalizationRegistry currently only releases pinner IDs without cleaning up the underlying Go objects. Other feedback suggests adding argument length checks in the C++ wrappers for better stability and replacing hardcoded session IDs with dynamic management.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a Node-API wrapper for the Spanner shared library, providing a bridge between Node.js and the Go-based library. Feedback focuses on critical memory management issues, including the incorrect use of FinalizationRegistry for short-lived identifiers and missing or redundant pinner ID releases in the native and TypeScript layers. Additionally, improvements are needed for C++ argument validation and the removal of hardcoded session identifiers to ensure compatibility with real Spanner environments.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a Node-API wrapper for the Spanner shared library, featuring C++ bindings and TypeScript abstractions for database management. The review identifies several high-severity issues, including native memory leaks in the C++ worker classes and an architectural flaw where Go objects are unpinned but not properly closed. Additional feedback suggests improving safety through argument validation, removing unused parameters, and avoiding tight coupling to internal package paths.