WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.46k stars 396 forks source link

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

Open broccolihighkicks opened 1 year ago

broccolihighkicks commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.