Skip to content

Load onnxruntime-node only when no runtime is injected - #1814

Open
iliev-blnk wants to merge 2 commits into
huggingface:mainfrom
iliev-blnk:lazy-onnxruntime-node
Open

iliev-blnk wants to merge 2 commits into
huggingface:mainfrom
iliev-blnk:lazy-onnxruntime-node

Conversation

@iliev-blnk

Copy link
Copy Markdown

Fixes #1813.

onnx-node.js called require('onnxruntime-node') at import time, so the bundled native binding loaded even when a runtime was injected through globalThis[Symbol.for('onnxruntime')]. Next to an app's own onnxruntime-node at another version, that crashes with version `VERS_1.30.0' not found.

onnx-node.js now exports a loader, and onnx.js calls it only when nothing is injected. The web build's esbuild stub for ./onnx-node.js is unaffected, since that branch never runs in the browser.

The old "Import order matters" comment dates from the shared ONNX registry in 2023 and no longer applies, so I removed it.

Tests

  • New tests/onnx_node_loading.test.js imports the package in a child process and checks whether onnxruntime_binding.node ends up in require.cache: loaded without injection, not loaded with it. The second case fails on main. It runs in a child process because jest.isolateModulesAsync hits "Module cache already has entry" for onnxruntime-common under ESM.
  • pnpm test: every suite passes locally except tests/models.test.js, which runs out of memory on my 15 GB machine on main as well. tests/pipelines.test.js passes 148/148 on both main and this branch.

With #1743

On its own this gets past the crash, but then the empty device list from #1743 shows up (Unsupported device: "cpu". Should be one of: .). With both, the repro in #1813 prints [ 1, 3, 384 ] on the app's onnxruntime-node 1.24.3. The two conflict on one line of onnx.js; the resolution is ONNX = injectedONNX ?? loadOnnxNode();.

This branch has not been deployed

No deployments
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.

Injecting a runtime via Symbol.for('onnxruntime') still loads the bundled onnxruntime-node, which crashes next to a different onnxruntime-node version

1 participant