Bite.ts 41/update t encrypt structure#300
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Emscripten build and Node package layout for @skalenetwork/t-encrypt to resolve module-loading issues in ESM/bundler environments (e.g., React), by producing separate Node and Web WASM bundles and adjusting package exports accordingly.
Changes:
- Split the Emscripten
encrypttarget intoencrypt_nodeandencrypt_webwith environment-specific linker flags. - Update Node package
exports/published files to include both Node and Web artifacts and add a Node-ESM entrypoint. - Update test scripts and the Emscripten test runner to use
encrypt_nodeartifacts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| threshold_encryption/CMakeLists.txt | Builds separate encrypt_node/encrypt_web targets and copies outputs into the Node package folder. |
| scripts/run_emscripten_test.sh | Copies encrypt_node.* into the test build dir instead of encrypt.*. |
| test/test.js | Uses encrypt_node.js for Node-based Emscripten testing. |
| test/test2Keys.js | Uses encrypt_node.js for Node-based Emscripten testing. |
| node/package.json | Bumps version, adds conditional exports for browser vs node, and updates published artifacts list. |
| node/EncryptMessageNode.mjs | Adds a Node-ESM wrapper that loads encrypt_node and attempts to locate the wasm. |
| node/EncryptMessage.mjs | Switches browser wrapper to encrypt_web and adds a cached module initializer + wasm URL resolution. |
| node/EncryptMessage.js | Switches Node CJS wrapper to encrypt_node and adds cached module initializer + wasm path resolution. |
| add_custom_command(TARGET encrypt_node POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_node.js ${CMAKE_SOURCE_DIR}/node/encrypt_node.js | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_node.wasm ${CMAKE_SOURCE_DIR}/node/encrypt_node.wasm | ||
| COMMENT "Copying encrypt_node to node package folder" | ||
| ) | ||
| add_custom_command(TARGET encrypt_web POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_web.js ${CMAKE_SOURCE_DIR}/node/encrypt_web.js | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_web.wasm ${CMAKE_SOURCE_DIR}/node/encrypt_web.wasm | ||
| COMMENT "Copying encrypt_web to node package folder" |
There was a problem hiding this comment.
The POST_BUILD steps copy generated artifacts into ${CMAKE_SOURCE_DIR}/node, which mutates the source tree and can break read-only source checkouts or multi-config/out-of-tree builds (multiple build dirs racing to overwrite the same files). Prefer copying into an install/package staging directory (e.g., via install(FILES ...) + cpack, or a dedicated package_node target that writes under ${CMAKE_CURRENT_BINARY_DIR}) and have the publish step pull from that output.
| add_custom_command(TARGET encrypt_web POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_web.js ${CMAKE_SOURCE_DIR}/node/encrypt_web.js | ||
| COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/encrypt_web.wasm ${CMAKE_SOURCE_DIR}/node/encrypt_web.wasm | ||
| COMMENT "Copying encrypt_web to node package folder" |
There was a problem hiding this comment.
These POST_BUILD commands write build outputs into ${CMAKE_SOURCE_DIR}/node, which mutates the source tree during a build and can fail in read-only source checkouts (or cause dirty working trees in CI). Consider copying into the build tree and/or making this step opt-in (e.g., behind an option or an explicit install/packaging target).
fixes #246 skalenetwork/bite-ts#41