Skip to content
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

Using better-sqlite3 in a global CLI causes errors. #1015

Open
broccolihighkicks opened this issue May 30, 2023 · 5 comments
Open

Using better-sqlite3 in a global CLI causes errors. #1015

broccolihighkicks opened this issue May 30, 2023 · 5 comments

Comments

@broccolihighkicks
Copy link

I have a Node.js CLI that I am developing that is installed with npm install --global my-cli

This is in my package.json:

"bin": {
    "my-cli": "dist/cli.js"
}

The issue is that when my-cli is run, it is run with from the current working directory where it is called.

better-sqlite3 seems to try to read the SQLite compiled lib based on the current working directory, causing it to fail with these errors:

1. Error: Could not find module root given file: "node:internal/modules/cjs/loader". Do you have a `package.json` file?
2. Error: Could not locate the bindings file. Tried:

I have a temporary work around where I set the current working directory before new Database and then restore it afterwards.

What is the correct config for a global CLI that uses better-sqlite3?

Also, how are prebuilt binaries used? On an M1 it seems to build SQLite from source, is this normal?

@neoxpert
Copy link
Contributor

neoxpert commented Jun 9, 2023

Please give more information. Which versions of NodeJS and better-sqlite3 are you using?

In the end it does not matter if you install a project in the global context to use it as CLI or using it as a simple, local application. In any case better-sqlite3 requires a native module to be present. As long as you are using a NodeJS version, for which a prebuilt binary is present, it would automatically be downloaded when installing your module. If there is no fitting prebuilt binary available it will try to build the native module on demand, as you are observing on your system.

@broccolihighkicks
Copy link
Author

broccolihighkicks commented Jun 9, 2023

Versions:

v18.9.0 node 
"better-sqlite3": "^8.4.0"

I think you misunderstand my issue - maybe as I asked two questions.

The issue is that I am building my own CLI using Node, and when it is installed and run, better-sqlite uses the current-working-directory of where the CLI is started to look for its built binaries.

This is an error, because when my CLI is installed with npm install --global my-cli, better-sqlite should be looking for the dynamic SQLite lib/DDL in the global node_modules (/opt/homebrew/lib/node_modules), not the current-working-directory where I am running my CLI.

# To find the global NPM node_modules:
npm root -g
/opt/homebrew/lib/node_modules

My temp fix is to set the Node process CWD to /opt/homebrew/lib/node_modules when opening a db file with better-sqlite (so that it looks in the right place for the SQLite dynamic lib), and then setting it back afterwards.

@NathanaelA
Copy link

The PR #974 should allow you to fix this issue easily in your code, this is basically the same issue that Electron sees, where paths are totally messed up. For normal Node.JS, everything tends to works great but in these edge cases where the native .node file will be in a different location, #974 will allow you to load it yourself and pass it in. ;-)

@neoxpert
Copy link
Contributor

Electron by itself does not mess up paths. It does not know about the lifecycle, packaging and special cases of each individual app using it. You have to make the paths work in your build process. Also, this particular issue is not related to Electron.

After some hours of playing around with a custom module, installed as CLI, using another native module I would tend to say, the way loading modules in NodeJS is a bit quirky, especially if bindings (the module) is used too. The best solution so far was looking up the global module folder path and looking up the absolute path to the native module - I hope there is a better solution, but so far it does not seem this is an issue a library can solve every potential case of usage.

@NathanaelA
Copy link

Issue's with native .node modules in Electron has many, many corner cases! Especially if you are trying to elk out a fast startup. There are many hidden gotcha's with everything from several of the bundling system auto-replacing "require" with their own "require" function to the libraries assuming where to load its .node file from. Throw in having a node module version is different (#126) than electron, GLIBC issues (like #990) and you have a fun area to navigate when using any .node module with Electron. Some of these are Electron specific issues, some of them are bundling issues, some of them are as you say just the nature of how electron / node handles paths...


I've got an entire build process (including app signing) for Windows and Linux that fixes all the above issues, but #974 will allow me to eliminate using a custom version of better-sqlite3 JS code.

In better-sqlite3, if you use vite/rollup or webpack, the "require" is replaced with its own "require" function that doesn't understand how to load a .node file like is used here https://github.com/WiseLibs/better-sqlite3/blob/master/lib/database.js#L50
and will just ignore these path's from bindings since everything is assumed to be "bundled".
https://github.com/TooTallNate/node-bindings/blob/master/bindings.js#L36-L59

So then if you DON'T externalize in your bundler: bindings, file-uri-to-path & better-sqlite3 then when better-sqlite3 attempts to load the "better-sqlite3.node" file, it just can't find it because the path IS messed up. This is why using #974 can help fix this issue, because you can pre-load the .node file yourself and pass it in since you know exactly where it lives in a release version of an Electron app and everything will be bundled properly.

I've customized my own copy of the better-sqlite3 -- I have the following change in my code which I pass into better-sqlite:

 // Production only, as we have to load the .node file from next to where the app.asar file lives
 if (import.meta.env.PROD) {
      options.nativeBinding = __filename.substring(0, __filename.indexOf("app.asar")) + "better_sqlite3.node";
    }

Then some custom code to the better sqlite database.js that uses options.nativeBinding if it is defined it is to use a dynamically generated require statement (i.e. bypassing the rollup/webpack one) and then load my passed path's better_sqlite3.node file.

const create = nodeModule.createRequire || nodeModule.createRequireFromPath;
    const requireMe = create( typeof document === "undefined" ? require("url").pathToFileURL(__filename).href : document.currentScript && document.currentScript.src || new URL("index.js", document.baseURI).href );
		addon = requireMe(path.resolve(nativeBindingPath).replace(/(\.node)?$/, '.node'));

Using #974, I will be able to revert back to the stock better-sqlite library and eliminate the final better-sqlite3 electron issue that I've run into. Which is basically what you are running into, not having the proper path to the .node file. You can preload the .node file and pass it in and be done. ;-)

I've also had to do this same stuff with another module also (although that one I just forked and maintain my own copy as it never changes). By doing these steps I've saved probably 200-300ms in startup speed by eliminating all the extra extraction, disk searching hits, and finally eliminating loading each separate JS file since I only load a single bundled JS file for all database stuff.

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

No branches or pull requests

4 participants