diff --git a/.github/workflows/build-node-wrapper/action.yml b/.github/workflows/build-node-wrapper/action.yml index 9d2f14d59f..32aa251ea3 100644 --- a/.github/workflows/build-node-wrapper/action.yml +++ b/.github/workflows/build-node-wrapper/action.yml @@ -79,7 +79,11 @@ runs: rm -rf node_modules && rm -rf package-lock.json && npm install cd rust-client rm -rf node_modules && rm -rf package-lock.json && npm install - + - name: npm install for /npm/glide package + shell: bash + working-directory: ./node/npm/glide + run: | + rm -rf node_modules && rm -rf package-lock.json && npm install - name: Build shell: bash working-directory: ./node diff --git a/.github/workflows/node-create-package-file/action.yml b/.github/workflows/node-create-package-file/action.yml index 8c9cc9d2f2..294e90de47 100644 --- a/.github/workflows/node-create-package-file/action.yml +++ b/.github/workflows/node-create-package-file/action.yml @@ -85,3 +85,27 @@ runs: envsubst < package.json.tmpl > "package.json" cat package.json echo $(ls *json*) + - name: Create package.json file in npm/glide package + shell: bash + working-directory: ./node/npm/glide + run: | + name="valkey-glide" + export node_os="${{ inputs.named_os }}" + export node_arch="${{ inputs.arch }}" + echo "The workflow is: ${{env.EVENT_NAME}}" + if ${{ env.EVENT_NAME == 'workflow_dispatch' }}; then + R_VERSION="${{ env.INPUT_VERSION }}" + else + R_VERSION=${GITHUB_REF:11} + fi + echo "RELEASE_VERSION=${R_VERSION}" >> $GITHUB_ENV + export package_version=${R_VERSION} + export scope=`if [ "${{ inputs.npm_scope }}" != '' ]; then echo "${{ inputs.npm_scope }}/"; fi` + export MUSL_FLAG=`if [[ "${{ inputs.target }}" =~ .*"musl".* ]]; then echo "-musl"; fi` + export pkg_name="${name}-${node_os}${MUSL_FLAG}-${node_arch}" + export dev_dependency_name="${scope}${pkg_name}" + # Create package.json and append devDependency + mv package.json package.json.tmpl + envsubst < package.json.tmpl > "package.json" + jq --arg dev_dependency_name "$dev_dependency_name" --arg path "../.." '.devDependencies += {($dev_dependency_name): $path}' package.json > package.tmpl.json && mv package.tmpl.json package.json + cat package.json diff --git a/node/DEVELOPER.md b/node/DEVELOPER.md index 9568e3be82..34df6e41d0 100644 --- a/node/DEVELOPER.md +++ b/node/DEVELOPER.md @@ -16,14 +16,14 @@ Software Dependencies If your Nodejs version is below the supported version specified in the client's [documentation](https://github.com/valkey-io/valkey-glide/blob/main/node/README.md#nodejs-supported-version), you can upgrade it using [NVM](https://github.com/nvm-sh/nvm?tab=readme-ov-file#install--update-script). -- npm -- git -- GCC -- pkg-config -- protoc (protobuf compiler) -- openssl -- openssl-dev -- rustup +- npm +- git +- GCC +- pkg-config +- protoc (protobuf compiler) +- openssl +- openssl-dev +- rustup **Dependencies installation for Ubuntu** @@ -107,16 +107,16 @@ Before starting this step, make sure you've installed all software requirments. 5. Integrating the built GLIDE package into your project: Add the package to your project using the folder path with the command `npm install /node`. -- For a fast build, execute `npm run build`. This will perform a full, unoptimized build, which is suitable for developing tests. Keep in mind that performance is significantly affected in an unoptimized build, so it's required to build with the `build:release` or `build:benchmark` option when measuring performance. -- If your modifications are limited to the TypeScript code, run `npm run build-external` to build the external package without rebuilding the internal package. -- If your modifications are limited to the Rust code, execute `npm run build-internal` to build the internal package and generate TypeScript code. -- To generate Node's protobuf files, execute `npm run build-protobuf`. Keep in mind that protobuf files are generated as part of full builds (e.g., `build`, `build:release`, etc.). +- For a fast build, execute `npm run build`. This will perform a full, unoptimized build, which is suitable for developing tests. Keep in mind that performance is significantly affected in an unoptimized build, so it's required to build with the `build:release` or `build:benchmark` option when measuring performance. +- If your modifications are limited to the TypeScript code, run `npm run build-external` to build the external package without rebuilding the internal package. +- If your modifications are limited to the Rust code, execute `npm run build-internal` to build the internal package and generate TypeScript code. +- To generate Node's protobuf files, execute `npm run build-protobuf`. Keep in mind that protobuf files are generated as part of full builds (e.g., `build`, `build:release`, etc.). > Note: Once building completed, you'll find the compiled JavaScript code in the `node/build-ts` folder. ### Troubleshooting -- If the build fails after running `npx tsc` because `glide-rs` isn't found, check if your npm version is in the range 9.0.0-9.4.1, and if so, upgrade it. 9.4.2 contains a fix to a change introduced in 9.0.0 that is required in order to build the library. +- If the build fails after running `npx tsc` because `glide-rs` isn't found, check if your npm version is in the range 9.0.0-9.4.1, and if so, upgrade it. 9.4.2 contains a fix to a change introduced in 9.0.0 that is required in order to build the library. ### Test @@ -202,13 +202,13 @@ Development on the Node wrapper may involve changes in either the TypeScript or **TypeScript:** -- ESLint -- Prettier +- ESLint +- Prettier **Rust:** -- clippy -- fmt +- clippy +- fmt #### Running the linters @@ -231,8 +231,8 @@ Development on the Node wrapper may involve changes in either the TypeScript or ### Recommended extensions for VS Code -- [Prettier - Code formatter](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode) - JavaScript / TypeScript formatter. -- [ESLint](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) - linter. -- [Jest Runner](https://marketplace.visualstudio.com/items?itemName=firsttris.vscode-jest-runner) - in-editor test runner. -- [Jest Test Explorer](https://marketplace.visualstudio.com/items?itemName=kavod-io.vscode-jest-test-adapter) - adapter to the VSCode testing UI. -- [rust-analyzer](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust-analyzer) - Rust language support for VSCode. +- [Prettier - Code formatter](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode) - JavaScript / TypeScript formatter. +- [ESLint](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) - linter. +- [Jest Runner](https://marketplace.visualstudio.com/items?itemName=firsttris.vscode-jest-runner) - in-editor test runner. +- [Jest Test Explorer](https://marketplace.visualstudio.com/items?itemName=kavod-io.vscode-jest-test-adapter) - adapter to the VSCode testing UI. +- [rust-analyzer](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust-analyzer) - Rust language support for VSCode. diff --git a/node/README.md b/node/README.md index eeaf4e4a60..299499d202 100644 --- a/node/README.md +++ b/node/README.md @@ -14,16 +14,16 @@ The release of Valkey GLIDE was tested on the following platforms: Linux: -- Ubuntu 22.04.1 (x86_64 and aarch64) -- Amazon Linux 2023 (AL2023) (x86_64) +- Ubuntu 22.04.1 (x86_64 and aarch64) +- Amazon Linux 2023 (AL2023) (x86_64) macOS: -- macOS 14.7 (Apple silicon/aarch_64) +- macOS 14.7 (Apple silicon/aarch_64) Alpine: -- node:alpine (default on aarch64 and x86_64) +- node:alpine (default on aarch64 and x86_64) ## NodeJS supported version diff --git a/node/npm/glide/index.ts b/node/npm/glide/index.ts index a7fad935c1..7ed7a3da80 100644 --- a/node/npm/glide/index.ts +++ b/node/npm/glide/index.ts @@ -4,71 +4,37 @@ * Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ -import { GLIBC, MUSL, familySync } from "detect-libc"; +import { GLIBC, familySync } from "detect-libc"; import { arch, platform } from "process"; let globalObject = global as unknown; /* eslint-disable @typescript-eslint/no-require-imports */ function loadNativeBinding() { - let nativeBinding = null; + let nativeStr = process.env.native_binding; - switch (platform) { - case "linux": - switch (arch) { - case "x64": - switch (familySync()) { - case GLIBC: - nativeBinding = require("@scope/valkey-glide-linux-x64"); - break; - case MUSL: - nativeBinding = require("@scope/valkey-glide-linux-musl-x64"); - break; - default: - nativeBinding = require("@scope/valkey-glide-linux-x64"); - break; - } + if (nativeStr == undefined) { + const prefix = familySync() == GLIBC ? "" : "-musl"; + nativeStr = `${platform}${prefix}-${arch}`; - break; - case "arm64": - switch (familySync()) { - case GLIBC: - nativeBinding = require("@scope/valkey-glide-linux-arm64"); - break; - case MUSL: - nativeBinding = require("@scope/valkey-glide-linux-musl-arm64"); - break; - default: - nativeBinding = require("@scope/valkey-glide-linux-arm64"); - break; - } - - break; - default: - throw new Error( - `Unsupported OS: ${platform}, architecture: ${arch}`, - ); - } - - break; - case "darwin": - switch (arch) { - case "arm64": - nativeBinding = require("@scope/valkey-glide-darwin-arm64"); - break; - default: - throw new Error( - `Unsupported OS: ${platform}, architecture: ${arch}`, - ); - } - - break; - default: + if ( + !["x64", "arm64"].includes(arch) || + !["linux", "darwin"].includes(platform) + ) { throw new Error( `Unsupported OS: ${platform}, architecture: ${arch}`, ); + } } + let scope = process.env.scope || "@scope"; + + if (scope == "@scope") { + scope = "@valkey/"; + } + + const nativeBinding = require(`${scope}valkey-glide-${nativeStr}`); + if (!nativeBinding) { throw new Error(`Failed to load native binding`); } diff --git a/node/npm/glide/jest.config.js b/node/npm/glide/jest.config.js new file mode 100644 index 0000000000..0a0eec9ef4 --- /dev/null +++ b/node/npm/glide/jest.config.js @@ -0,0 +1,32 @@ +/* eslint no-undef: off */ +module.exports = { + preset: "ts-jest", + transform: { + "^.+\\.(t|j)s$": ["ts-jest", { isolatedModules: true }], + }, + testEnvironment: "node", + testRegex: "/tests/.*\\.(test|spec)?\\.(ts|tsx)$", + moduleFileExtensions: [ + "ts", + "tsx", + "js", + "jsx", + "json", + "node", + "cjs", + "mjs", + ], + testTimeout: 600000, + reporters: [ + "default", + [ + "./node_modules/jest-html-reporter", + { + includeFailureMsg: true, + includeSuiteFailure: true, + executionTimeWarningThreshold: 60, + sort: "status", + }, + ], + ], +}; diff --git a/node/npm/glide/package.json b/node/npm/glide/package.json index 8444de8ef9..c678768d98 100644 --- a/node/npm/glide/package.json +++ b/node/npm/glide/package.json @@ -1,5 +1,5 @@ { - "name": "${scope}${pkg_name}", + "name": "valkey-glide", "types": "build-ts/index.d.ts", "version": "${package_version}", "description": "General Language Independent Driver for the Enterprise (GLIDE) for Valkey", @@ -33,6 +33,11 @@ }, "homepage": "https://github.com/valkey-io/valkey-glide#readme", "devDependencies": { + "@jest/globals": "^29.7.0", + "@types/jest": "^29.5.14", + "ts-jest": "^29.2.5", + "jest": "^29.7.0", + "jest-html-reporter": "^3.10.2", "@types/node": "^18.11.18", "@typescript-eslint/eslint-plugin": "^5.48.0", "@typescript-eslint/parser": "^5.48.0", diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts new file mode 100644 index 0000000000..7cd4220f15 --- /dev/null +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -0,0 +1,196 @@ +/** + * Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 + */ + +import { it } from "@jest/globals"; +import * as f from "fs/promises"; +import { describe } from "node:test"; +import * as ts from "typescript"; +import * as glideApi from "../"; //ESM convention, + +const skippedListForExports: string[] = [ + "AdvancedBaseClientConfiguration", + "ClusterScanOptions", + "GlideMultiJson", +]; + +const glideRsKeyWords: string[] = [ + "ClusterScanCursor", + "Script", + "createLeakedArray", + "createLeakedAttribute", + "createLeakedBigint", + "createLeakedDouble", + "createLeakedMap", + "createLeakedString", + "default", +]; + +const skipFolders = [ + "commonjs-test", + "glide-logs", + "hybrid-node-tests", + "node_modules", + "npm", + ".cargo", + "target", + "tests", +]; + +describe("Validation of Exported Symbols", () => { + it("check excluded symbols are not exported", async () => { + // Check exported symbols for valkey glide package + const exportedSymbolsList = Object.keys(glideApi).sort(); // exportedList from the npm/glide package. + + const implBuildFolder = "./build-ts/"; + const filesWithNodeCode = await getFiles(implBuildFolder); + + const internallyExported: string[] = []; + + for (const file of filesWithNodeCode) { + const sourceCode = await f.readFile(file, "utf8"); + const sourceFile = await ts.createSourceFile( + file, + sourceCode, + ts.ScriptTarget.Latest, + true, + ); + internallyExported.push(...visitRoot(sourceFile)); + } + + internallyExported.sort(); + + const missingSymbols = internallyExported.filter( + (e: string) => + !exportedSymbolsList.includes(e) && + !skippedListForExports.includes(e), + ); + + const doesNotExistExports = exportedSymbolsList.filter( + (e: string) => + !internallyExported.includes(e) && !glideRsKeyWords.includes(e), + ); + + if (missingSymbols.length > 0) { + console.log( + "The following symbols are exported from npm/glide package but missing " + + "from the internal node package export. These symbols might be from glide-rs package", + ); + console.log(missingSymbols); + } + + expect(missingSymbols.length).toBe(0); + + if (doesNotExistExports.length > 0) { + console.log( + "Symbols that might be missed from the npm/glide package export:", + ); + console.log(doesNotExistExports); + } + + expect(doesNotExistExports.length).toBe(0); + }); +}); + +async function getFiles(folderName: string): Promise { + const files = await f.readdir(folderName, { withFileTypes: true }); + + const filesWithNodeCode = []; + + for (const file of files) { + if (file.isDirectory()) { + if (skipFolders.includes(file.name)) { + continue; + } + + filesWithNodeCode.push( + ...(await getFiles(folderName + file.name + "/")), + ); + } else { + if (!file.name.endsWith(".d.ts")) { + continue; + } + + filesWithNodeCode.push(folderName + file.name); + } + } + + return filesWithNodeCode; +} + +function visitRoot(root: ts.Node) { + // (Root Level)->(Level 1) + const children: ts.Node[] = root.getChildren(); + const resultList: string[] = []; + + // (Root Level) -> (Level 1) -> Level 2. This is the level in the AST where all the exported symbols in a file are present. + for (const node of children) { + const nodeList: string[] = node + .getChildren() + .map((c) => visit(c)) + .filter((c) => c !== undefined); + + if (nodeList.length > 0) { + resultList.push(...nodeList); + } + } + + return resultList; +} + +function visit(node: ts.Node) { + let name: string | undefined = ""; + + // List of exported symbols we want to ignore. + switch (node.kind) { + case ts.SyntaxKind.FirstStatement: + case ts.SyntaxKind.ExportDeclaration: + case ts.SyntaxKind.ExportAssignment: + case ts.SyntaxKind.ImportDeclaration: + return; + } + + // list exported symbols we want to check for, like, InterfaceDeclaration, FunctionDeclaration, etc. + if (ts.isFunctionDeclaration(node)) { + name = node.name?.text; + } else if (ts.isVariableStatement(node)) { + name = ""; + } else if (ts.isInterfaceDeclaration(node)) { + name = node.name?.text; + } else if (ts.isClassDeclaration(node)) { + name = node.name?.text; + } else if (ts.isTypeAliasDeclaration(node)) { + name = node.name?.text; + } else if (ts.isEnumDeclaration(node)) { + name = node.name?.text; + } else if (ts.isModuleDeclaration(node)) { + name = node.name?.text; + } + + const children = node.getChildren(); + const isInternal = children + .find((c) => ts.SyntaxKind[c.kind] == "JSDocComment") + ?.getText() + .includes("@internal"); + const isExported = children + .find((c) => ts.SyntaxKind[c.kind] == "SyntaxList") + ?.getChildren() + .find((c) => ts.SyntaxKind[c.kind] == "ExportKeyword"); + + if (isExported && !isInternal) { + // Not internal symbol exported for external use. + return name; + } + + if (isExported && isInternal) { + // marked correctly... no-op. Exported for internal use in the code. + } + + if (!isExported && isInternal) { + // no-op + } + + if (!isExported && !isInternal) { + // no-op + } +} diff --git a/node/npm/glide/tests/tsconfig.json b/node/npm/glide/tests/tsconfig.json new file mode 100644 index 0000000000..2a8aaebf40 --- /dev/null +++ b/node/npm/glide/tests/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "rootDir": "../" + }, + "include": ["./*.test.ts"] +} diff --git a/node/rust-client/src/lib.rs b/node/rust-client/src/lib.rs index 584eab16de..109e8cfae5 100644 --- a/node/rust-client/src/lib.rs +++ b/node/rust-client/src/lib.rs @@ -311,6 +311,7 @@ fn split_pointer(pointer: *mut T) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -321,6 +322,9 @@ pub fn create_leaked_string(message: String) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test +/// This function is for tests that require a value allocated on the heap. +/// Should NOT be used in production. pub fn create_leaked_string_vec(message: Vec) -> [u32; 2] { // Convert the string vec -> Bytes vector let bytes_vec: Vec = message.iter().map(|v| Bytes::from(v.to_vec())).collect(); @@ -329,6 +333,7 @@ pub fn create_leaked_string_vec(message: Vec) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -342,6 +347,7 @@ pub fn create_leaked_map(map: HashMap) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -353,6 +359,7 @@ pub fn create_leaked_array(array: Vec) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -368,6 +375,7 @@ pub fn create_leaked_attribute(message: String, attribute: HashMap [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 025d0218bf..b3e8f0b855 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -729,6 +729,10 @@ export interface PubSubMsg { */ export type WritePromiseOptions = RouteOption & DecoderOption; +/** + * @internal + * Base client interface for GLIDE + */ export class BaseClient { private socket: net.Socket; protected readonly promiseCallbackFunctions: diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 97c36ce694..17d6750ef9 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -54,7 +54,7 @@ function toBuffersArray(args: GlideString[]) { } /** - * @internal + * @internal @test */ export function parseInfoResponse(response: string): Record { const lines = response.split("\n"); @@ -434,6 +434,7 @@ export function createHGet( } /** + * @internal * This function converts an input from {@link HashDataType} or `Record` types to `HashDataType`. * * @param fieldsAndValues - field names and their values. diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 460c32ff82..6393789292 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -287,7 +287,7 @@ import { command_request } from "./ProtobufMessage"; * console.log(result); // Output: ['OK', 'value'] * ``` */ -export class BaseTransaction> { +class BaseTransaction> { /** * @internal */ diff --git a/node/src/server-modules/GlideFtOptions.ts b/node/src/server-modules/GlideFtOptions.ts index 6d9e8f4528..e5c16e0e7d 100644 --- a/node/src/server-modules/GlideFtOptions.ts +++ b/node/src/server-modules/GlideFtOptions.ts @@ -58,7 +58,7 @@ export type VectorField = BaseField & { /** * Base class for defining vector field attributes to be used after the vector algorithm name. */ -export interface VectorFieldAttributes { +interface VectorFieldAttributes { /** Number of dimensions in the vector. Equivalent to `DIM` in the module API. */ dimensions: number; /**