fix(packages/js): add node js support via worker_threads - #433
fix(packages/js): add node js support via worker_threads#433Prachi-Gupta2808 wants to merge 16 commits into
worker_threads#433Conversation
Co-authored-by: Ryan Millard <142347829+Ryan-Millard@users.noreply.github.com>
github-actions
Bot
added
js/ts
example-app
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds Node.js runtime support to the ChangesNode.js Runtime Support and Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/src/pages/index.jsx`:
- Around line 47-59: The client-side useEffect currently calls
fetch("https://api.github.com/repos/Ryan-Millard/Img2Num") which risks hitting
GitHub's unauthenticated rate limit; change this to avoid direct browser calls
by either (A) moving the GitHub API call into a build-time step (e.g.,
Docusaurus data fetch during site build) and populate setStats with embedded
static data, or (B) create a small serverless/backend endpoint that the
useEffect calls instead (e.g., /api/github-stats) which queries GitHub with an
authenticated token and short-TTL caching (5–15 minutes) before returning JSON;
update the component to call that endpoint from useEffect and keep setStats
logic the same.
- Around line 47-59: The useEffect that calls
fetch("https://api.github.com/repos/Ryan-Millard/Img2Num") should use an
AbortController to avoid calling setStats after the component unmounts: create
an AbortController inside the useEffect, pass controller.signal to fetch, check
the fetch response and only call setStats when the request was not aborted, and
in the effect cleanup call controller.abort(); also handle the abort error in
the catch (ignore DOMException "AbortError") so it doesn't surface.
In `@example-apps/console-js/index.js`:
- Around line 1-13: Add concise, user-facing comments to this example: explain
the imports (writeFileSync and imageToSvg), document the pixels
Uint8ClampedArray as an RGBA 1x1 white pixel and why values are
[255,255,255,255], annotate width and height variables, comment the imageToSvg({
pixels, width, height }) call (including that it returns { svg } and uses
await), and note the writeFileSync('output.svg', svg) line saves the SVG to
disk; also keep the console.log lines as brief explanations of each step for
newcomers.
In `@packages/js/wasmWorker.js`:
- Around line 223-234: The Node path assumes parentPort exists; add a defensive
null check around parentPort before using it: after importing parentPort in the
isNode branch, verify parentPort is truthy and only then set postMsg = (data) =>
parentPort.postMessage(data) and call parentPort.on('message', ...) to invoke
handleMessage; if parentPort is null, set postMsg to a safe fallback (noop or an
error-throwing function) and/or throw a clear Error indicating this module must
run in a worker, so postMsg, parentPort and handleMessage usage are guarded.
🪄 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: 359a1fa1-b9cb-4cff-ba83-aea795260c1f
⛔ Files ignored due to path filters (3)
docs/static/img/homepage-demo.pngis excluded by!**/*.pngdocs/static/img/homepage-demo.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (12)
bindings/js/CMakeLists.txtdocs/docusaurus.config.jsdocs/src/components/Hedgehog.jsxdocs/src/pages/index.jsxdocs/src/pages/index.mddocs/src/pages/index.module.cssexample-apps/console-js/index.jsexample-apps/console-js/package.jsonpackages/js/safeWasmWrappers.jspackages/js/wasmClient.jspackages/js/wasmWorker.jspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- docs/src/pages/index.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.txt
📄 CodeRabbit inference engine (.editorconfig)
**/*.txt: Do not enforce maximum line length for text files
Use 2-space indentation for text files
Files:
bindings/js/CMakeLists.txt
bindings/**
⚙️ CodeRabbit configuration file
bindings/**: Language bindings for the Img2Num library. Ensure the public API surface matches
the core C/C++ headers and that error propagation is handled correctly for each
binding language. Also ensure the relevant docstrings are present to enable
auto-generation of documentation.
Files:
bindings/js/CMakeLists.txt
**
⚙️ 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: Google
IndentWidth: 4
ColumnLimit: 100
AllowShortFunctionsOnASingleLine: false
**: 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_length = 120-------------------------
C++ / C headers
-------------------------
[*.{hpp,cpp,c,h}]
indent_style = space
indent_size = 4
charset = utf-8
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true
max_line_length = 120-------------------------
CMake
-------------------------
[CMakeLists.txt]
indent_style = space
indent_size = 2
charset = utf-8
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true
max_line_length = 120[*.cmake]
indent_style = space
indent_size = 2
charset = utf-8
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true
max_line_length = 120-------------------------
Ja...
Files:
bindings/js/CMakeLists.txtpnpm-workspace.yamlexample-apps/console-js/package.jsondocs/src/components/Hedgehog.jsxexample-apps/console-js/index.jspackages/js/safeWasmWrappers.jsdocs/docusaurus.config.jspackages/js/wasmClient.jsdocs/src/pages/index.jsxpackages/js/wasmWorker.jsdocs/src/pages/index.module.css
**/*.json
📄 CodeRabbit inference engine (.editorconfig)
Do not trim trailing whitespace in JSON files
Files:
example-apps/console-js/package.json
example-apps/**
⚙️ CodeRabbit configuration file
example-apps/**: - Example applications. These are for demonstration; keep them minimal and ensure
they correctly reflect the public API. Flag any use of internal/private APIs.
- IMPORTANT: These example apps must be a good reflection of how to use Img2Num's
library, which means that they need good comments and must have clean code. This
is meant to be external and is designed for others to use to get started.
Files:
example-apps/console-js/package.jsonexample-apps/console-js/index.js
**/*.{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:
docs/src/components/Hedgehog.jsxexample-apps/console-js/index.jspackages/js/safeWasmWrappers.jsdocs/docusaurus.config.jspackages/js/wasmClient.jsdocs/src/pages/index.jsxpackages/js/wasmWorker.js
docs/**/*.{js,jsx,ts,tsx,mdx}
⚙️ CodeRabbit configuration file
docs/**/*.{js,jsx,ts,tsx,mdx}: This is the Docusaurus site source. Check for MDX compatibility, correct
plugin usage, and valid imports. Review docusaurus.config.js changes carefully
for broken routing or plugin config regressions.
Files:
docs/src/components/Hedgehog.jsxdocs/docusaurus.config.jsdocs/src/pages/index.jsx
**/*.css
📄 CodeRabbit inference engine (.editorconfig)
Use 2-space indentation for CSS files
Files:
docs/src/pages/index.module.css
🧠 Learnings (1)
📚 Learning: 2026-03-14T17:55:21.677Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 278
File: example-apps/react-js/src/components/GlassModal.module.css:35-43
Timestamp: 2026-03-14T17:55:21.677Z
Learning: CSS Module files (*.module.css) should use camelCase identifiers for class names and keyframe names (e.g., fadeIn, scaleIn). Regular (vanilla) CSS files should use kebab-case. This convention helps distinguish module-scoped identifiers from global styles. Apply this rule to all CSS Module files across the repository (and similarly structured projects); for non-module CSS, continue using kebab-case.
Applied to files:
docs/src/pages/index.module.css
🔇 Additional comments (15)
docs/src/components/Hedgehog.jsx (1)
42-42: LGTM!Also applies to: 64-65
docs/docusaurus.config.js (1)
99-111: LGTM!docs/src/pages/index.jsx (1)
10-38: LGTM!Also applies to: 61-120, 122-175, 177-225, 227-238, 241-308, 310-323
docs/src/pages/index.module.css (1)
1-595: LGTM!bindings/js/CMakeLists.txt (1)
25-25: LGTM!packages/js/wasmClient.js (4)
63-66: LGTM!
68-69: LGTM!
81-98: LGTM!
71-79: Node.js Worker ESM mode is already set viapackages/js/package.json
packages/js/wasmWorker.jsis underpackages/js/, wherepackages/js/package.jsonsets"type": "module", sonew Worker(.../wasmWorker.js)already loads it as ESM;{ type: 'module' }is redundant here.> Likely an incorrect or invalid review comment.packages/js/wasmWorker.js (2)
157-159: LGTM!
161-221: LGTM!packages/js/safeWasmWrappers.js (2)
19-22: LGTM!
24-241: LGTM!example-apps/console-js/package.json (1)
1-11: LGTM!pnpm-workspace.yaml (1)
4-4: LGTM!
| import { writeFileSync } from 'fs'; | ||
| import { imageToSvg } from 'img2num'; | ||
|
|
||
| // 1x1 white pixel PNG for testing | ||
| const pixels = new Uint8ClampedArray([255, 255, 255, 255]); | ||
| const width = 1; | ||
| const height = 1; | ||
|
|
||
| console.log('Running img2num in Node.js...'); | ||
| const { svg } = await imageToSvg({ pixels, width, height }); | ||
| console.log('SVG output:', svg); | ||
| writeFileSync('output.svg', svg); | ||
| console.log('Saved to output.svg'); No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add educational comments to the example application.
Example apps are external-facing and designed to help others get started. Adding comments explaining each step would improve the learning experience for users new to the library.
📚 Suggested improvements
+/**
+ * Example: Using img2num in Node.js
+ *
+ * This example demonstrates how to use the img2num library in a Node.js
+ * environment to convert a raster image to SVG format.
+ */
import { writeFileSync } from 'fs';
import { imageToSvg } from 'img2num';
-// 1x1 white pixel PNG for testing
+// Create a minimal test image: 1x1 white pixel
+// In real usage, you would load an actual image file
const pixels = new Uint8ClampedArray([255, 255, 255, 255]);
const width = 1;
const height = 1;
console.log('Running img2num in Node.js...');
+
+// Convert the raster image to SVG
+// imageToSvg applies bilateral filtering, K-means clustering, and contour detection
const { svg } = await imageToSvg({ pixels, width, height });
+
console.log('SVG output:', svg);
+
+// Write the result to a file
writeFileSync('output.svg', svg);
console.log('Saved to output.svg');Based on coding guidelines: example apps "must have good comments and must have clean code. This is meant to be external and is designed for others to use to get started."
📝 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.
| import { writeFileSync } from 'fs'; | |
| import { imageToSvg } from 'img2num'; | |
| // 1x1 white pixel PNG for testing | |
| const pixels = new Uint8ClampedArray([255, 255, 255, 255]); | |
| const width = 1; | |
| const height = 1; | |
| console.log('Running img2num in Node.js...'); | |
| const { svg } = await imageToSvg({ pixels, width, height }); | |
| console.log('SVG output:', svg); | |
| writeFileSync('output.svg', svg); | |
| console.log('Saved to output.svg'); | |
| /** | |
| * Example: Using img2num in Node.js | |
| * | |
| * This example demonstrates how to use the img2num library in a Node.js | |
| * environment to convert a raster image to SVG format. | |
| */ | |
| import { writeFileSync } from 'fs'; | |
| import { imageToSvg } from 'img2num'; | |
| // Create a minimal test image: 1x1 white pixel | |
| // In real usage, you would load an actual image file | |
| const pixels = new Uint8ClampedArray([255, 255, 255, 255]); | |
| const width = 1; | |
| const height = 1; | |
| console.log('Running img2num in Node.js...'); | |
| // Convert the raster image to SVG | |
| // imageToSvg applies bilateral filtering, K-means clustering, and contour detection | |
| const { svg } = await imageToSvg({ pixels, width, height }); | |
| console.log('SVG output:', svg); | |
| // Write the result to a file | |
| writeFileSync('output.svg', svg); | |
| console.log('Saved to output.svg'); |
🤖 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 `@example-apps/console-js/index.js` around lines 1 - 13, Add concise,
user-facing comments to this example: explain the imports (writeFileSync and
imageToSvg), document the pixels Uint8ClampedArray as an RGBA 1x1 white pixel
and why values are [255,255,255,255], annotate width and height variables,
comment the imageToSvg({ pixels, width, height }) call (including that it
returns { svg } and uses await), and note the writeFileSync('output.svg', svg)
line saves the SVG to disk; also keep the console.log lines as brief
explanations of each step for newcomers.
Source: Coding guidelines
| // -------- Environment-aware message binding -------- | ||
| // Use worker_threads in Node.js, Web Worker API in browser | ||
| let postMsg; | ||
|
|
||
| if (isNode) { | ||
| const { parentPort } = await import('worker_threads'); | ||
| postMsg = (data) => parentPort.postMessage(data); | ||
| parentPort.on('message', (data) => handleMessage(data)); | ||
| } else { | ||
| postMsg = (data) => self.postMessage(data); | ||
| self.onmessage = ({ data }) => handleMessage(data); | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding a null check for parentPort in Node.js worker context.
In Node.js worker_threads, parentPort is null when code runs in the main thread (outside a worker). While this file is only loaded as a Worker, a defensive null check would prevent confusing errors if accidentally imported in the main thread.
🛡️ Proposed defensive check
if (isNode) {
const { parentPort } = await import('worker_threads');
+ if (!parentPort) {
+ throw new Error('wasmWorker.js must run in a worker thread, not the main thread');
+ }
postMsg = (data) => parentPort.postMessage(data);
parentPort.on('message', (data) => handleMessage(data));
} else {📝 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.
| // -------- Environment-aware message binding -------- | |
| // Use worker_threads in Node.js, Web Worker API in browser | |
| let postMsg; | |
| if (isNode) { | |
| const { parentPort } = await import('worker_threads'); | |
| postMsg = (data) => parentPort.postMessage(data); | |
| parentPort.on('message', (data) => handleMessage(data)); | |
| } else { | |
| postMsg = (data) => self.postMessage(data); | |
| self.onmessage = ({ data }) => handleMessage(data); | |
| } | |
| // -------- Environment-aware message binding -------- | |
| // Use worker_threads in Node.js, Web Worker API in browser | |
| let postMsg; | |
| if (isNode) { | |
| const { parentPort } = await import('worker_threads'); | |
| if (!parentPort) { | |
| throw new Error('wasmWorker.js must run in a worker thread, not the main thread'); | |
| } | |
| postMsg = (data) => parentPort.postMessage(data); | |
| parentPort.on('message', (data) => handleMessage(data)); | |
| } else { | |
| postMsg = (data) => self.postMessage(data); | |
| self.onmessage = ({ data }) => handleMessage(data); | |
| } |
🤖 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/wasmWorker.js` around lines 223 - 234, The Node path assumes
parentPort exists; add a defensive null check around parentPort before using it:
after importing parentPort in the isNode branch, verify parentPort is truthy and
only then set postMsg = (data) => parentPort.postMessage(data) and call
parentPort.on('message', ...) to invoke handleMessage; if parentPort is null,
set postMsg to a safe fallback (noop or an error-throwing function) and/or throw
a clear Error indicating this module must run in a worker, so postMsg,
parentPort and handleMessage usage are guarded.
Ryan-Millard
changed the title
worker_threads
Ryan-Millard
left a comment
Ryan-Millard
left a comment
There was a problem hiding this comment.
Hi @Prachi-Gupta2808. This is great stuff! Thank you.
I just have a few changes I'd like implemented here before this gets merged. This looks like a good fix.
Additionally, I'd also like @Krasner to review this because I'm having some trouble with my computers at the moment and can't run anything on them.
I look forward to merging this. Thanks again!🦔
|
@Ryan-Millard & @Krasner I did the rest changes, except one, I looked at how the Python example uses Which package would you guys be okay with? |
|
|
Hi @Prachi-Gupta2808, @Krasner's suggestion sounds good (Node doesn't have it built-in). Please just make sure it gets the Uint8ClampedArray in It should conceptually be an array like this: [
255, // Red
255, // Green
255, // Blue
255, // Alpha
...
]; |
|
Hi @Ryan-Millard & @Krasner , The pipeline seems to be hanging after the WebGPU fallback messages, even on a tiny 20x20 image. It prints the two WebGPU warning messages and then just hangs indefinitely without producing any output or error. Is the CPU fallback expected to work in Node.js or is there something else needed to get it running? |
I can take a look later today. Can you show me how to run the node code? I assume it's the console-js example |
|
The cpu fallback is just standard wasm c++ |
Hi! Yes it's the console-js example. To run it: # From repo root inside Docker
pnpm install
# Navigate to the example app
cd example-apps/console-js
# Run with an image path
node index.js <image-path>
# Example
node index.js test.jpgIt prints the WebGPU fallback warnings and then hangs after that without producing output. |
Would it help if I add some console.log statements to trace where exactly it's getting stuck? |
That would be helpful |
|
There are also 3 merge conflicts on this PR. Can you address them? |
|
Have you tried any of the other example apps on your computer to verify that it's working? It could also be your hardware that's a bit slow (especially on a CPU). Please would you give me the details of your hardware so I can confirm what's going on. |
|
@Ryan-Millard it's just not showing up in the diff viewer: |
I think it's the best solution because
I was talking about what I broke. I'm not sure what I broke.😭 |
Oh, you changed webgpu from peerDependency to dependency and also removed it from example-apps/console-js/package.json in this commit 93576f2 but you're right it would be better to have a pure node.js build with webgpu as a real dependency, and then a browser build (which is the react-app as we have now) without it |
|
Hi. Here's an update from my side: Unfortunately, I didn't realise the large scope that the issue related to this actually has and it would be pretty unfair for me to expect @Prachi-Gupta2808 to get it all done alone because it's not a very newcomer-friendly issue and she isn't very familiar with the codebase. As a result, I will probably need to work on this PR a bit to set up the bundling since @Krasner isn't very experienced with it (unless you would like to try it out - I don't mind). With that, I'll probably only be able to work on it on Wednesday (or later). What needs to be done:
Additionally, the introduction of Node has made #391 all the more relevant because newcomers now have to battle the various build systems individually:
Thank you both (@Prachi-Gupta2808 and @Krasner) for your work on this pull request! |
Please would you explain how the opaque error is caused by |
Check PR #439 |
I will look into other issues and would contribute in them if I find something interesting! |
|
In case there is something I can help you guys with, please let me know. |
Thank you. I don't think that there is much more that you can do on this issue because it'll involve some complex things that I'll have to set up. Thank you for the wonderful work you've done on it. I'll try to get this PR merged soon. |
|
Hey @Prachi-Gupta2808, @Krasner and I have been really impressed with your work so far. This PR and the nice docs contributions are great. @Krasner and I are considering adding you as a maintainer (mainly for the docs/ and JS side for now). Would you be interested in that? |
|
Hiii @Ryan-Millard and @Krasner , I would love that! But as you guys already know, I am new to this so I might be a little too much of a "Oh I have a doubt" person for you both. One more thing, can you guys give me your linkedin or slack invite link maybe? I did try searching you guys. That way I can ask you both any doubt if I have, only if you both would be interested in helping me a little. Thank youuu! |
No problem, we all have different levels of experience in different areas. The project is growing so it's really nice to have some help. We prefer to keep most, if not all, conversations on github - through issues, discussions, or PRs so that they are visible to all new comers. If you have any questions, issues or discussions would be a good place for them. In fact this will be a good test to check if we can help new comers get up to speed. Let us know your experience contributing to this repo. Was it documented enough to understand how this repo functions? (We're working on updating the documentation) |
|
Thank you so much, I'd love to! 🦔 The docs were pretty easy to follow especially coming in with some React experience. Setup-wise it was also smooth. The one thing I had to ask about was the WASM build step ( Apart from that the codebase is well commented and easy to navigate. I'll spend today and tomorrow reading through more of the documentation properly ( I want to learn more about the project) and will share more detailed feedback after that! |
PR #439 should make that much clearer. We are introducing a |
|
Hi @Prachi-Gupta2808. I'm to hear that you're interested in joining us. Welcome to the team.🥳🦔 Outsider perspectives are important to us and we appreciate the insight about the WebAssembly setup. We definitely need to overhaul our documentation because it's been neglected for too long. @Krasner maybe we should set this up in #439 with something like |
|
Hi @Krasner. I'm a bit stuck with this implementation because it doesn't seem like there is a clean way to implement it across the browser and node environments - the bundler is causing problems because it embeds the worker as a base64 string in the output file (for efficiency), which causes problems when calling the WebAssembly. On the Node side of things, it needs a relative path, which Vite breaks when it builds because it restructures everything. Using the current approach, I cannot seem to get both environments to work simultaneously - getting node to work breaks the browser code and vice versa. I'm trying my best to have separate builds and isolate the node- and browser-specific imports so they aren't included in the final outputs for both (e.g., Overall, I think it may actually be a better idea to remove the workers entirely from the library because they aren't really necessary and cause additional work for us for no reason. By removing it, we would be transferring the responsibility onto the user, which could be a good and a bad thing at the same time:
I'd like to know your thoughts on the matter. I think this would be a good idea because we don't handle threading and stuff like that for the other libraries - they are expected to deal with it themselves. |
|
When you have some time, please take a look at this blog post: We're Expanding the Maintainer Team After reading it, could you let us know which areas of the project you feel most comfortable working on? We'd like to use that information when assigning issues and planning future work. Also, are you familiar with reviewing pull requests on GitHub? Reviewing code is an important part of being a maintainer. If you're not sure how the process works, this short video gives a good overview: https://www.youtube.com/watch?v=lSnbOtw4izI&t=136s It would also be helpful if you could tell us a bit about your current experience and interests. For example:
There's no expectation that you'll be experienced in all of these areas. Knowing where you're most confident will help us assign suitable tasks and update our CODEOWNERS file over time. Finally, since you're new to maintaining open-source projects, please don't hesitate to ask questions (I don't find lots of questions annoying). We expect questions, and we'd much rather have discussions happen publicly in issues, discussions, and pull requests where other contributors can benefit from them too. Welcome to the team!🦔🦔🦔🦔 |
|
@Ryan-Millard , Thanks for the warm welcome! No, I currently don't have any familiarity with reviewing PRs on github but would catch up to it, thank you for the resource. I'll mostly be comfortable with:
Outside of this I really want to work on Documentation and Technical writing because I am an author too and like it just makes me feel curious about trying it. I want to give it a shot. Apart from this I wish to learn testing and CI too but I really don't want to make img2num my "figuring it out project" with lots of new experiments, I think I can do it over time. |
If you ever need any help with it, let me know. I'd be happy to help you.
That sounds good - those are our main languages (we mainly use JS for scripting). We don't currently use TypeScript in this project because the benefits of it aren't really felt in our use cases.
Docusaurus is a very good one because a lot of projects use it for their documentation because it's simple to use. I really want to upgrade the documentation site because it's still using the default Docusaurus configuration, but I have no time for it - you're free to work on that if you'd like, but it's not an important feature.
I dislike how GitHub set this stuff up. It's very difficult to test most of the time. For simpler workflows, I usually test them out on a private repository that I don't care about so I don't have to worry about thousands of mistakes on this repository. In the end, nothing is truly testable and you just have to YOLO it sometimes.😅😅 |
|
@Ryan-Millard do you have a brach with your Node experiments that I can take a look at? |
https://github.com/Ryan-Millard/Img2Num/tree/wip/pr-433/fix/node-js-support |
|
#449 is the succeeds this PR. |
3 participants
What was changed & why
The
img2numJS package used the browser-only Web Workers API, causing aReferenceError: Worker is not definedcrash in any Node.js environment. This PR adds Node.js support by detecting the runtime environment and usingworker_threadsinstead of Web Workers when running in Node.js.Fixes: #394
Changes
packages/js/wasmClient.js: added environment detection; usesworker_threadsWorker in Node.js, existing Web Worker in browser ( Didn't change anything in this flow)packages/js/wasmWorker.js: extracted message handler intohandleMessage(); usesparentPortfor messaging in Node.js, existingself.onmessage/self.postMessagein browser (Didn't change anything for browser here either)bindings/js/CMakeLists.txt: addednodetoENVIRONMENTflag so the compiled WASM supports Node.js runtimeexample-apps/console-js: new example app demonstrating the library working in a Node.js environmentTesting & Verification
Tested inside the Docker container (
mainimage as mentioned by @Ryan-Millard ):Worker is not definederrorENVIRONMENTflagnode index.jsinexample-apps/console-js, library loaded and processed successfully in Node.jsBrowser path is completely untouched so existing functionality is unaffected.
Additional Resources
Note: I already mentioned the webGPU one stuff.
Thank you!