fix: react-app and node build - #449
Conversation
Co-authored-by: Ryan Millard <142347829+Ryan-Millard@users.noreply.github.com>
…2808/Img2Num into fix/node-js-support
IMPORTANT: Node is still broken - I (@Ryan-Millard) will fix it in a follow-up in this PR. React example works fine.
|
Warning Review limit reached
More reviews will be available in 51 minutes and 48 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe JS package now builds separate browser and Node.js artifacts, routes worker setup through target-specific helpers, adds a Node console example, and updates build, CI, and documentation tooling to match the new package layout. ChangesNode.js target for img2num JS package
Sequence Diagram(s)sequenceDiagram
participant Caller
participant wasmClient
participant createWorker
participant wasmWorker
participant WebGPU
Caller->>wasmClient: imageToSvg(...)
wasmClient->>createWorker: await createWorker()
createWorker-->>wasmClient: worker wrapper
wasmClient->>wasmWorker: postMessage({ id, funcName, args })
wasmWorker->>wasmWorker: handleMessage(data)
wasmWorker-->>wasmClient: postMessage({ id, output, returnValue })
wasmClient-->>Caller: resolve SVG result
alt Node target
wasmWorker->>WebGPU: initWebGPU()
wasmWorker->>WebGPU: destroyWebGPU()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
c/c++
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/js/src/wasmClient.js`:
- Around line 87-94: The worker.onError handler currently rejects pending
callbacks and clears them, but does not reset the internal state (the
`initialized` flag and `worker` reference remain unchanged). This causes
subsequent calls to route to the dead worker instead of reinitializing. After
calling callbacks.clear() in the error handler, reset `initialized` to false and
clear or terminate the `worker` reference so that future operations will
properly reinitialize the worker.
In `@packages/js/src/workers/wasmWorker.js`:
- Around line 235-238: The message handler in `parentPort.on("message", async
(data) => {...})` is closing the worker port and destroying WebGPU after
handling each individual message, but the worker is designed to handle multiple
requests until explicitly terminated. Remove the `destroyWebGPU()` and
`parentPort.close()` calls from the message handler so the worker remains
available for subsequent calls to `callWasm()`. The cleanup logic should only
execute when the worker is explicitly terminated through the
`terminateWasmWorker()` function, not after every message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a2f0922b-b12b-4670-8d8a-0cc84a9436da
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (10)
.github/workflows/build-react-app.ymlJustfileeslint.config.jspackages/js/src/target/browser/worker.jspackages/js/src/target/node/webgpu.jspackages/js/src/target/node/worker.jspackages/js/src/wasmClient.jspackages/js/src/workers/wasmWorker.jspackages/js/tsconfig.typedoc.jsonpackages/js/vite.config.js
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Lint & Validate Code
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.editorconfig)
**/*.{js,ts,jsx,tsx}: Use 2-space indentation for JavaScript and TypeScript files
Maintain 200 character maximum line length for JavaScript/TypeScript files
Files:
packages/js/src/target/node/worker.jspackages/js/src/target/browser/worker.jseslint.config.jspackages/js/src/workers/wasmWorker.jspackages/js/src/target/node/webgpu.jspackages/js/src/wasmClient.jspackages/js/vite.config.js
**
⚙️ CodeRabbit configuration file
**: # Contributing to Img2NumWant to contribute to Img2Num? There are a few things you need to know.
We wrote a contribution guide to help you get started.
A few important points:
- Add tests with your PR — new features and bug fixes must include tests where appropriate. PRs without tests are unlikely to be approved.
- Follow the repository's coding style rules.
- Use the issue and PR templates when filing issues or submitting code. Your PR will be rejected if you don't.
If you're unsure what to change, open a discussion and someone will assist you.
Questions?
If you have questions or need help:
- Open a discussion
- Create an issue
- Check existing PRs for ideas
Thank you for improving Img2Num! 🎨🚀
**: BasedOnStyle: LLVM
Standard: c++20--- Basic formatting ---
IndentWidth: 4
ColumnLimit: 100
TabWidth: 4--- Braces ---
Cpp11BracedListStyle: true
SpaceBeforeCpp11BracedList: true
BreakBeforeBraces: Attach--- Braced initializers ---
Cpp11BracedListStyle: true
--- Constructor initializer lists ---
PackConstructorInitializers: Never
BreakConstructorInitializers: BeforeComma
ConstructorInitializerIndentWidth: 4--- Alignment ---
AlignAfterOpenBracket: BlockIndent
--- Pointers ---
PointerAlignment: Left
--- Includes ---
IncludeBlocks: Regroup
SortIncludes: CaseInsensitive--- Extern "C" cleanliness ---
IndentExternBlock: NoIndent
--- Lambdas ---
AllowShortLambdasOnASingleLine: Inline
--- Functions ---
AllowShortFunctionsOnASingleLine: None
**: root = true-------------------------
Global defaults
-------------------------
[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
max_line_lengt...
Files:
packages/js/src/target/node/worker.jspackages/js/src/target/browser/worker.jseslint.config.jspackages/js/tsconfig.typedoc.jsonpackages/js/src/workers/wasmWorker.jspackages/js/src/target/node/webgpu.jspackages/js/src/wasmClient.jspackages/js/vite.config.jsJustfile
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: GitHub Actions workflows. Review for:
- SHA-pinned action versions for third-party actions (security best practice).
- Secrets accessed only via ${{ secrets.* }} — never hardcoded.
- Least-privilege permissions on each job/workflow.
- Correct job dependency ordering (needs:) and if/condition logic.
Files:
.github/workflows/build-react-app.yml
**/*.json
📄 CodeRabbit inference engine (.editorconfig)
Do not trim trailing whitespace in JSON files
Files:
packages/js/tsconfig.typedoc.json
🧠 Learnings (2)
📚 Learning: 2026-05-01T22:50:11.527Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 339
File: release-please-config.json:18-47
Timestamp: 2026-05-01T22:50:11.527Z
Learning: In this repo, release-please-action v4 preserves '/' verbatim in slash-containing path-based package keys when emitting GitHub Actions output names (e.g., `bindings/c--release_created`). When referencing these step outputs in `job.outputs` (and other expressions), use bracket notation with the exact output name: `${{ steps.release.outputs['bindings/c--release_created'] }}` rather than dot notation. If needed, map the complex step output to a clean job-level output alias so downstream jobs can use dot notation via that alias.
Applied to files:
.github/workflows/build-react-app.yml
📚 Learning: 2026-05-19T17:30:09.565Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 375
File: .github/workflows/cmake-build.yml:86-88
Timestamp: 2026-05-19T17:30:09.565Z
Learning: In Ryan-Millard/Img2Num CI/workflow YAMLs, any `uv sync` command used for the Python package build must include `--no-build-isolation` (do not remove it). If you need deterministic dependency installs for CI, you may add `--frozen` alongside it (e.g., `uv sync --frozen --no-build-isolation`), and it should not conflict with the repo’s build setup.
Applied to files:
.github/workflows/build-react-app.yml
🪛 ast-grep (0.43.0)
packages/js/src/target/node/webgpu.js
[warning] 33-33: Avoid using the initial state variable in setState
Context: setTimeout(resolve, 50)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🔇 Additional comments (8)
Justfile (1)
109-109: LGTM!.github/workflows/build-react-app.yml (1)
28-28: LGTM!Also applies to: 38-39
packages/js/tsconfig.typedoc.json (1)
13-21: LGTM!eslint.config.js (1)
18-18: LGTM!packages/js/vite.config.js (1)
4-4: LGTM!packages/js/src/target/browser/worker.js (1)
7-7: LGTM!packages/js/src/target/node/worker.js (1)
3-3: LGTM!Also applies to: 17-17
packages/js/src/target/node/webgpu.js (1)
31-35: LGTM!
| worker.onError((event) => { | ||
| const output = event.message || "WASM worker error"; | ||
| const err = new Error(`[Img2Num wasmClient] Error: ${output}`); | ||
| for (const [_id, cb] of callbacks) { | ||
| cb.reject(err); | ||
| } | ||
| callbacks.clear(); | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Reset worker state after error to allow recovery.
After Line 87 error handling runs, pending promises are rejected, but initialized stays true and worker remains set. Subsequent calls can route to a dead worker instead of reinitializing. Clear runtime state in the error path (and terminate if still alive).
Suggested fix
worker.onError((event) => {
const output = event.message || "WASM worker error";
const err = new Error(`[Img2Num wasmClient] Error: ${output}`);
for (const [_id, cb] of callbacks) {
cb.reject(err);
}
callbacks.clear();
+ worker?.terminate();
+ worker = null;
+ initialized = false;
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/js/src/wasmClient.js` around lines 87 - 94, The worker.onError
handler currently rejects pending callbacks and clears them, but does not reset
the internal state (the `initialized` flag and `worker` reference remain
unchanged). This causes subsequent calls to route to the dead worker instead of
reinitializing. After calling callbacks.clear() in the error handler, reset
`initialized` to false and clear or terminate the `worker` reference so that
future operations will properly reinitialize the worker.
| parentPort.on("message", async (data) => { | ||
| await handleMessage(data); | ||
| await destroyWebGPU(); | ||
| parentPort.close(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Worker teardown per message
rg -n -C3 'parentPort\.on\("message"|destroyWebGPU\(\)|parentPort\.close\(\)' packages/js/src/workers/wasmWorker.js
# Client-side persistent lifecycle expectations
rg -n -C4 'initialized|initWasmWorker|terminateWasmWorker|worker\.postMessage' packages/js/src/wasmClient.jsRepository: Ryan-Millard/Img2Num
Length of output: 3319
Do not close the worker after handling each message.
Lines 237–238 tear down GPU and close the worker port after every message, but wasmClient initializes the worker once and expects it to handle multiple requests until terminateWasmWorker() is explicitly called. This creates a lifecycle mismatch: subsequent calls to callWasm() will fail or hang against a closed worker.
The worker should remain open for the duration of its lifetime and only shut down when explicitly instructed by the client.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/js/src/workers/wasmWorker.js` around lines 235 - 238, The message
handler in `parentPort.on("message", async (data) => {...})` is closing the
worker port and destroying WebGPU after handling each individual message, but
the worker is designed to handle multiple requests until explicitly terminated.
Remove the `destroyWebGPU()` and `parentPort.close()` calls from the message
handler so the worker remains available for subsequent calls to `callWasm()`.
The cleanup logic should only execute when the worker is explicitly terminated
through the `terminateWasmWorker()` function, not after every message.
There was a problem hiding this comment.
what is your suggestion?
There was a problem hiding this comment.
@Krasner, here is the concrete fix. Move cleanup out of the per-message handler and into a parentPort.on("close", ...) listener. That event fires when the parent calls worker.terminate() (i.e., terminateWasmWorker()), so WebGPU is destroyed exactly once at the end of the worker's lifetime instead of after every request.
Current code (problematic):
parentPort.on("message", async (data) => {
await handleMessage(data);
await destroyWebGPU(); // ← tears down GPU after every message
parentPort.close(); // ← closes the port after every message
});Suggested fix:
parentPort.on("message", async (data) => {
await handleMessage(data);
// Worker stays alive for subsequent callWasm() requests
});
// Clean up when the parent terminates the worker via worker.terminate()
parentPort.on("close", async () => {
await destroyWebGPU();
});This keeps the worker reusable for the full lifecycle managed by wasmClient.js, and cleanup still fires exactly once when terminateWasmWorker() is called from the parent side.
Note on the console-js example: If that app sends exactly one message and immediately exits, the per-message teardown appeared to "work" locally — but it would break any Node consumer that calls
imageToSvgmore than once. The fix above is safe for both single- and multi-call scenarios.
There was a problem hiding this comment.
when I use
parentPort.on("close", async () => {
await destroyWebGPU();
});
never gets called
There was a problem hiding this comment.
@Krasner, that's a known Node.js gotcha. When the parent calls worker.terminate(), the thread is killed forcefully — parentPort.on("close", ...) is not guaranteed to fire before termination, so async cleanup never runs.
The reliable pattern is a sentinel shutdown message. The parent sends a special message before terminating, the worker runs its async cleanup and then closes itself.
packages/js/src/workers/wasmWorker.js — replace the per-message teardown:
parentPort.on("message", async (data) => {
if (data?.__shutdown === true) {
// Ordered async teardown triggered by the parent
try {
await destroyWebGPU();
} finally {
parentPort.close(); // now it's the worker closing its own port — fires reliably
}
return;
}
await handleMessage(data);
});packages/js/src/wasmClient.js — update terminateWasmWorker() to send the sentinel before terminating:
export async function terminateWasmWorker() {
if (!worker) return;
// Signal the worker to run async cleanup
worker.postMessage({ __shutdown: true });
// Give the worker a moment to finish cleanup, then hard-terminate as a fallback
await new Promise((resolve) => setTimeout(resolve, 200));
worker.terminate();
worker = null;
}This way:
- The shutdown message arrives before the thread is killed.
- The worker runs
destroyWebGPU()and closes the port on its own terms. - The 200 ms timeout is a safety net in case the GPU teardown hangs (matching the known NAPI crash described in the PR).
For the console-js single-shot app, calling
terminateWasmWorker()afterimageToSvg()resolves will trigger the same clean path — no special casing needed.
(ʕ·ᴥ·ʔ)? ← curious about whether the GPU crash still surfaces after this
There was a problem hiding this comment.
@Krasner I think that this should be in the src/target/*/worker.js files because they have terminate functions. I'm not quite sure what is going on here. It sucks that we have to include the WebGPU stuff in the library and that Node doesn't have it at all.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
@Ryan-Millard there is still some weird gpu deallocation issue with node js. I put a PR review on my own branch: Krasner#5 With this I think I reliably don't get any more gpu deallocation errors. |
|
Hi @Krasner. I'll have a look at this when I get a chance. I'm just currently busy with fixing the workflow failures on |
No worries! |
@Ryan-Millard can you review this? I need your insight here |
I'll have a look at it when I get a chance. I think I'll be able to do it tomorrow. |
|
Hi @Krasner. I'm sorry for not looking at this yesterday - my power went out just as I woke up (good old South Africa). After having a look at it now, I agree that your solution does fix it and probably is the best option. I think it's a Dawn teardown issue that may be coming from it freeing up it's own resources (internally) before RAII ever gets the chance to. I think this is a problem to address after this PR:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Justfile`:
- Around line 107-109: The console-js Just target currently runs
example-apps/console-js/index.js without guaranteeing the img2num JavaScript
package has been built first, so it can fail from a clean checkout. Update the
console-js recipe to depend on the existing JS build target used for the package
artifact (the same build step that satisfies react-js build), ensuring the JS
package is built before invoking the Node example.
- Around line 101-105: The echoed command in the Justfile is stale and does not
match the executable actually run by the recipe. Update the echo in the
console-c recipe so it prints the same binary path used by the execution line,
matching the existing console-cpp pattern and the actual executable name in the
console-c block. Keep the echoed command and the invoked command in sync so
copy-paste works correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67c1a8e8-e5db-48c3-9cfc-23f6024acbdb
📒 Files selected for processing (1)
Justfile
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Lint & Validate Code
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
**: # Contributing to Img2NumWant to contribute to Img2Num? There are a few things you need to know.
We wrote a contribution guide to help you get started.
A few important points:
- Add tests with your PR — new features and bug fixes must include tests where appropriate. PRs without tests are unlikely to be approved.
- Follow the repository's coding style rules.
- Use the issue and PR templates when filing issues or submitting code. Your PR will be rejected if you don't.
If you're unsure what to change, open a discussion and someone will assist you.
Questions?
If you have questions or need help:
- Open a discussion
- Create an issue
- Check existing PRs for ideas
Thank you for improving Img2Num! 🎨🚀
**: BasedOnStyle: LLVM
Standard: c++20--- Basic formatting ---
IndentWidth: 4
ColumnLimit: 100
TabWidth: 4--- Braces ---
Cpp11BracedListStyle: true
SpaceBeforeCpp11BracedList: true
BreakBeforeBraces: Attach--- Braced initializers ---
Cpp11BracedListStyle: true
--- Constructor initializer lists ---
PackConstructorInitializers: Never
BreakConstructorInitializers: BeforeComma
ConstructorInitializerIndentWidth: 4--- Alignment ---
AlignAfterOpenBracket: BlockIndent
--- Pointers ---
PointerAlignment: Left
--- Includes ---
IncludeBlocks: Regroup
SortIncludes: CaseInsensitive--- Extern "C" cleanliness ---
IndentExternBlock: NoIndent
--- Lambdas ---
AllowShortLambdasOnASingleLine: Inline
--- Functions ---
AllowShortFunctionsOnASingleLine: None
**: root = true-------------------------
Global defaults
-------------------------
[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
max_line_lengt...
Files:
Justfile
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Justfile`:
- Around line 107-109: The console-js Just target currently runs
example-apps/console-js/index.js without guaranteeing the img2num JavaScript
package has been built first, so it can fail from a clean checkout. Update the
console-js recipe to depend on the existing JS build target used for the package
artifact (the same build step that satisfies react-js build), ensuring the JS
package is built before invoking the Node example.
- Around line 101-105: The echoed command in the Justfile is stale and does not
match the executable actually run by the recipe. Update the echo in the
console-c recipe so it prints the same binary path used by the execution line,
matching the existing console-cpp pattern and the actual executable name in the
console-c block. Keep the echoed command and the invoked command in sync so
copy-paste works correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67c1a8e8-e5db-48c3-9cfc-23f6024acbdb
📒 Files selected for processing (1)
Justfile
🛑 Comments failed to post (2)
Justfile (2)
101-105: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Echo the actual executable names.
The logged commands are now out of sync with the binaries you execute, so anyone copying the echoed command gets the wrong path.
Suggested fix
console-cpp input: - `@echo` "./build-c-cpp/example-apps/console-cpp/console_cpp_app {{ input }}" + `@echo` "./build-c-cpp/example-apps/console-cpp/Img2NumExample_console_cpp {{ input }}" ./build-c-cpp/example-apps/console-cpp/Img2NumExample_console_cpp "{{ input }}" console-c input: - `@echo` "./build-c-cpp/example-apps/console-c/console_c_app {{ input }}" + `@echo` "./build-c-cpp/example-apps/console-c/CImg2NumExample_console_c {{ input }}" ./build-c-cpp/example-apps/console-c/CImg2NumExample_console_c "{{ input }}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.`@echo` "./build-c-cpp/example-apps/console-cpp/Img2NumExample_console_cpp {{ input }}" ./build-c-cpp/example-apps/console-cpp/Img2NumExample_console_cpp "{{ input }}" console-c input: `@echo` "./build-c-cpp/example-apps/console-c/Img2NumExample_console_c {{ input }}" ./build-c-cpp/example-apps/console-c/CImg2NumExample_console_c "{{ input }}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` around lines 101 - 105, The echoed command in the Justfile is stale and does not match the executable actually run by the recipe. Update the echo in the console-c recipe so it prints the same binary path used by the execution line, matching the existing console-cpp pattern and the actual executable name in the console-c block. Keep the echoed command and the invoked command in sync so copy-paste works correctly.
107-109: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make
console-jsbuild the JS package first.This target now runs the Node example without ensuring the
img2numnode artifact exists. From a clean checkout—or right afterjust clean packages-js—just console-js …can fail on the same missing-build contract that already affectedreact-js build.Suggested fix
-console-js input: +console-js input: build-packages-js `@echo` "node example-apps/console-js/index.js {{ input }}" node example-apps/console-js/index.js "{{ input }}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.console-js input: build-packages-js `@echo` "node example-apps/console-js/index.js {{ input }}" node example-apps/console-js/index.js "{{ input }}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` around lines 107 - 109, The console-js Just target currently runs example-apps/console-js/index.js without guaranteeing the img2num JavaScript package has been built first, so it can fail from a clean checkout. Update the console-js recipe to depend on the existing JS build target used for the package artifact (the same build step that satisfies react-js build), ensuring the JS package is built before invoking the Node example.
Yes go ahead! |
|
@coderabbitai please generate a commit message and description that I can use when I squash this PR into the Please also give me a full breakdown of the changes on this branch. |
|
Tip For best results, initiate chat on the files or code changes.
Suggested squash commit messageSuggested squash commit descriptionFull breakdown of the changes on this branch1. CI / workflow fixes
2. Justfile / developer workflow updates
3. WASM / native runtime targeting
4. JS package structure and publishing changes
5. Worker abstraction and WASM client refactor
6. Node-specific WebGPU lifecycle handling
This is the main runtime-side work that makes the same JS package usable in both environments. 7. React app bundling adjustments
This reduces browser build issues from Node-only modules leaking into the app bundle. 8. New Node example application
9. Workspace, docs, and linting updates
10. Test changes
Shorter alternative squash messageIf you want a slightly more release-oriented title: If you want, I can also turn this into:
|
|
@Prachi-Gupta2808 thank you again for the help with this! I'm glad that we finally found a solution and that everything works now. :) |
Update release.yml to support the post-#449 layout and fix the dev-branch release pipeline. - use dry-run-specific draft release tags for native, JS, and Python - update JS/WASM pipeline to use Node 24, PNPM 11.4.0, and pnpm publish - build WASM in a dedicated build-wasm directory with explicit CMake flags - package JS dist and WASM artifacts separately - add workspace allowBuilds entries required by PNPM - clarify that C bindings are required for WASM builds in CMakeLists.txt
3 participants
What was changed & why
@Ryan-Millard and @Prachi-Gupta2808 taking both of your work in #433 and fixing up the vite.config.js bundler will build both the browser and node packages.
will rebuild packages under
packages/js/dist/browserandpackages/js/dist/nodeto check:
both should now work. Let me know if this works for you too.
Still a gpu deallocation issue on the nodejs app that i'm not sure how to resolve yet
Fixes: #
Changes
Testing & Verification
Additional Resources