chore(spanner-driver): sync spannerlib-node code - #9025
Conversation
|
/gcbrun |
There was a problem hiding this comment.
Code Review
This pull request introduces a high-performance Node-API (N-API) wrapper for the Go-based Spanner shared library, establishing a three-layer pipeline that bridges TypeScript/JavaScript to the native Go SDK. It includes build configurations, shell scripts, C++ addon source code, TypeScript wrappers for connection and pool management, and a comprehensive test suite. Feedback on the changes highlights two issues: several devDependencies in package.json specify non-existent major versions that will cause installation failures, and a link in README.md contains a hardcoded local absolute file URL that should be updated to a relative path.
| "typescript": "^6.0.0", | ||
| "@types/node": "^24.0.0", | ||
| "@types/bindings": "^1.5.0", | ||
| "@types/mocha": "^10.0.6", | ||
| "@babel/core": "^8.0.0", | ||
| "@babel/cli": "^8.0.0", | ||
| "sinon": "^22.0.0", | ||
| "@types/sinon": "^21.0.0", | ||
| "gts": "^7.0.0", |
There was a problem hiding this comment.
Several devDependencies specify non-existent or hallucinated major versions (e.g., typescript v6.0.0, @types/node v24.0.0, @babel/core v8.0.0, @babel/cli v8.0.0, sinon v22.0.0, @types/sinon v21.0.0, and gts v7.0.0). These versions do not exist on the npm registry and will cause npm install to fail. Please downgrade them to the latest stable, existing versions.
| "typescript": "^6.0.0", | |
| "@types/node": "^24.0.0", | |
| "@types/bindings": "^1.5.0", | |
| "@types/mocha": "^10.0.6", | |
| "@babel/core": "^8.0.0", | |
| "@babel/cli": "^8.0.0", | |
| "sinon": "^22.0.0", | |
| "@types/sinon": "^21.0.0", | |
| "gts": "^7.0.0", | |
| "typescript": "^5.4.5", | |
| "@types/node": "^20.14.2", | |
| "@types/bindings": "^1.5.0", | |
| "@types/mocha": "^10.0.6", | |
| "@babel/core": "^7.24.5", | |
| "@babel/cli": "^7.24.5", | |
| "sinon": "^18.0.0", | |
| "@types/sinon": "^18.0.0", | |
| "gts": "^6.0.2", |
References
- Verify the existence of a package version on the npm registry (e.g. npmjs.com) before flagging it as a typo or non-existent.
- When managing dependencies in package.json, ensure that
@typespackages (e.g.,@types/sinon) are kept in sync with the major version of their corresponding main packages (e.g.,sinon).
| npm install | ||
| ``` | ||
|
|
||
| For a comprehensive architectural breakdown of the 3-layer compilation pipeline (Go Link, C++ node-gyp bridge, and Dual-publishing ESM/CJS), refer to the [BUILD_AND_RELEASE.md](file:///Users/gargsurbhi/Work/Github/go-sql-spanner/spannerlib/wrappers/spannerlib-node/BUILD_AND_RELEASE.md) documentation. |
There was a problem hiding this comment.
The link to BUILD_AND_RELEASE.md uses a hardcoded local absolute file:/// URL pointing to the author's local directory. This link will be broken for anyone else reading the documentation. Please change it to a relative link.
| For a comprehensive architectural breakdown of the 3-layer compilation pipeline (Go Link, C++ node-gyp bridge, and Dual-publishing ESM/CJS), refer to the [BUILD_AND_RELEASE.md](file:///Users/gargsurbhi/Work/Github/go-sql-spanner/spannerlib/wrappers/spannerlib-node/BUILD_AND_RELEASE.md) documentation. | |
| For a comprehensive architectural breakdown of the 3-layer compilation pipeline (Go Link, C++ node-gyp bridge, and Dual-publishing ESM/CJS), refer to the [BUILD_AND_RELEASE.md](BUILD_AND_RELEASE.md) documentation. |
|
/gcbrun |
Update the Spanner Node.js driver with the latest spannerlib-node code from go-sql-spanner.