-
-
Notifications
You must be signed in to change notification settings - Fork 0
Improve Node/Browser build #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6eecc5a to
78ff562
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for Node.js runtime and implements WebAssembly instantiation across different JavaScript runtimes (Deno, Node.js, Bun, and Browser). The changes refactor the runtime detection mechanism and introduce a unified interface for runtime-specific operations.
- Adds new
NodeRuntimeInterfaceclass to support Node.js environments - Introduces
instantiateWebAssemblymethod to the runtime interface for consistent WASM loading across all runtimes - Updates runtime detection to use
navigator.userAgentfor identifying Deno, Node.js, Bun, and browser environments
Reviewed Changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/js-runtime-compat/runtimes/node.ts | Introduces new NodeRuntimeInterface class (contains bugs - using Deno APIs instead of Node.js) |
| src/utils/js-runtime-compat/runtimes/deno.ts | Adds instantiateWebAssembly method using WebAssembly.instantiateStreaming |
| src/utils/js-runtime-compat/runtimes/browser.ts | Adds instantiateWebAssembly method using WebAssembly.instantiateStreaming |
| src/utils/js-runtime-compat/runtime.ts | Updates runtime detection to use navigator.userAgent and adds Node.js runtime handler |
| src/utils/js-runtime-compat/js-runtime-interface.ts | Extends interface with instantiateWebAssembly method and adds "node" and "bun" runtime types |
| src/datex-core/datex_core_js.js | Refactors to use runtimeInterface.instantiateWebAssembly instead of inline runtime detection |
| scripts/build-wasm.ts | Updates template to use runtimeInterface.instantiateWebAssembly |
| scripts/build-bundle.ts | Updates regex pattern for WASM replacement (pattern is too broad) |
| scripts/build-npm.ts | Removes dnt shims and unused entry points |
| src/datex-core/datex_core_js.d.ts | Reorders type declarations (cosmetic changes) |
| deno.lock | Adds @types/node dependency |
| Cargo.lock | Updates various Rust dependency versions |
Comments suppressed due to low confidence (1)
src/utils/js-runtime-compat/runtime.ts:44
- The runtime detection logic checks for "bun" runtime, but
getRuntimeInterfacedoesn't have a corresponding case to handle it. When Bun is detected, it will fall through to the browser runtime interface. Either add a Bun runtime implementation or remove "bun" from theJSRuntimeTypeand detection logic.
async function getRuntimeInterface(type: JSRuntimeType) {
if (type === "deno") {
const { DenoRuntimeInterface } = await import("./runtimes/deno.ts");
return new DenoRuntimeInterface();
} else if (type === "node") {
const { NodeRuntimeInterface } = await import("./runtimes/node.ts");
return new NodeRuntimeInterface();
} else {
const { BrowserRuntimeInterface } = await import(
"./runtimes/browser.ts"
);
return new BrowserRuntimeInterface();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removes Deno shims/polyfills for simpler and more stable node build that should also work in browsers.
Currently, only the websocket client interface is supported, we need to add a websocket server interface in the future.