Skip to content

Fix Node.js native library lookup in packaged Electron apps#797

Open
Andrey1994 wants to merge 1 commit intomasterfrom
codex/fix-issue-733-in-brainflow
Open

Fix Node.js native library lookup in packaged Electron apps#797
Andrey1994 wants to merge 1 commit intomasterfrom
codex/fix-issue-733-in-brainflow

Conversation

@Andrey1994
Copy link
Member

Motivation

  • Packaged Electron apps could not locate BrainFlow native libraries because the Node.js binding hardcoded __dirname/../brainflow/lib, which does not resolve inside app.asar/packaged layouts.
  • Provide a robust resolution strategy that works both in normal Node.js installs and in Electron production builds (including app.asar.unpacked and process.resourcesPath).

Description

  • Add a shared resolver resolveLibPath() in nodejs_package/brainflow/lib_path.ts that builds candidate paths (including unpacked app.asar and common node_modules locations) and returns the first existing path or the original default as a fallback.
  • Update the native loaders to use the new resolver by replacing the hardcoded path in BoardShim, DataFilter, and MLModel with resolveLibPath() in nodejs_package/brainflow/board_shim.ts, nodejs_package/brainflow/data_filter.ts, and nodejs_package/brainflow/ml_model.ts.
  • Add a small TypeScript-friendly cast to safely access process.resourcesPath when present in Electron.

Testing

  • Ran cd nodejs_package && npm run build and the TypeScript build completed successfully after adding the process cast. ✅
  • Ran cd nodejs_package && npm run lint and linting failed with many existing formatting issues unrelated to this change; the failure is due to pre-existing repository-wide prettier violations and not introduced by this patch. ❌
  • Verified the added resolver file nodejs_package/brainflow/lib_path.ts and that the three loaders now call resolveLibPath() instead of using the hardcoded __dirname path.

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 825105c228

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +16 to +18
const libPathCandidates = [
defaultLibPath,
...searchRoots.flatMap((root) => [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prioritize unpacked lib paths over app.asar paths

resolveLibPath() checks defaultLibPath first, so in Electron builds that use app.asar.unpacked it can still return an .../app.asar/... path when fs.existsSync reports that virtual ASAR entry as present. koffi.load/native dlopen requires a real filesystem path, so this selection causes native library loading to fail even when the unpacked copy exists; the unpacked candidates need to be searched before the default ASAR path.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant