Skip to content

Update index.js - #827

Open
BOT9315 wants to merge 1 commit into
ruvnet:mainfrom
BOT9315:patch-1
Open

Update index.js#827
BOT9315 wants to merge 1 commit into
ruvnet:mainfrom
BOT9315:patch-1

Conversation

@BOT9315

@BOT9315 BOT9315 commented Aug 13, 2026

Copy link
Copy Markdown

Updated the @ruvector/edge-full package to provide a unified and modular interface for all RuVector WASM components. The changes improve module initialization, add validation and error handling, clarify core vs. ONNX initialization, and fix issues in the quick-start example. The package now provides cleaner support for Edge, Graph, RVLite, SONA, DAG, and ONNX modules while making the initialization API easier and safer to use.

@ruvnet

ruvnet commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Thanks for the contribution — but this change can't be merged as-is: it makes the file syntactically invalid.

The patch adds a second export async function initAll() to a file that already declares one at line 91. The new declaration lands at line 113, inside the JSDoc block that documents a different function ("Initialize only specific modules"), so the original initAll is left in place rather than replaced.

Verified against the PR head:

$ grep -n 'export async function initAll' index.js
91:export async function initAll() {
113:export async function initAll() {

$ node --input-type=module --check < index.js
[stdin]:113
export async function initAll() {
       ^
SyntaxError: Identifier 'initAll' has already been declared

Duplicate top-level bindings are a hard error in ES modules, so nothing that imports this file will load — this isn't a lint nit, the module fails to parse.

It looks like the intent was to replace the existing initAll with a Promise.all-based version that initializes the five wasm modules concurrently. That's a reasonable improvement on its own merits. To land it:

  1. Delete the original initAll at line 91 instead of adding alongside it, and
  2. Re-attach the "Initialize only specific modules" JSDoc at line ~110 to the function it actually documents — right now the new code is wedged between that comment and its function.
  3. Confirm with node --input-type=module --check < examples/edge-full/pkg/index.js before pushing.

One thing worth checking before you invest more in it: examples/edge-full/pkg/ follows the wasm-pack output convention, so if this directory is regenerated by a build step, hand edits here would be overwritten and the change belongs in whatever generates it. A maintainer should confirm which it is — if it is checked-in generated output, the fix needs to go upstream of the generator instead.

Happy to re-review once it parses.

@ruvnet

ruvnet commented Aug 24, 2026

Copy link
Copy Markdown
Owner

The initModules rewrite here is a genuine improvement — the loader table is clearer than the switch, and if (typeof module.default === 'function') is more careful than the original. But the PR as it stands cannot be imported at all.

Blocking: duplicate initAll

The diff adds an export async function initAll() while the existing one is still in the file. On the PR head:

line  91: export async function initAll() {
line 113: export async function initAll() {
line 145: export async function initModules(moduleNames) {
$ node --input-type=module -e "import('./examples/edge-full/pkg/index.js')"
SyntaxError: Identifier 'initAll' has already been declared

This is a parse-time failure, so it takes the whole module down — initModules included, not just initAll. As a control, the same check against main gets past parsing and fails later at module resolution (expected without the built wasm artifacts), which is what a syntactically valid file looks like here.

The fix is to delete one of the two. The added copy and the existing one are the same Promise.all implementation, so removing the added block should be enough — worth a local node --check before pushing.

Minor: loaders inherits from Object.prototype

if (!loaders[name]) {
  throw new Error(`Unknown RuVector module: ${name}`);
}

loaders is an object literal, so loaders['constructor'], loaders['toString'] and friends resolve to inherited functions and are truthy. initModules(['constructor']) therefore skips the guard and calls Object() instead of reporting an unknown module — it won't crash, it just silently returns a bogus entry rather than the error the guard exists to produce.

Object.hasOwn(loaders, name) (or const loaders = Object.create(null)) closes it and keeps the intended behaviour for every real module name.

Happy to see this land once the duplicate is removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants