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

Bun support #208

Closed
rizrmd opened this issue Jan 19, 2023 · 6 comments
Closed

Bun support #208

rizrmd opened this issue Jan 19, 2023 · 6 comments

Comments

@rizrmd
Copy link

rizrmd commented Jan 19, 2023

Just tried installing lmdb with bun and running sample code:

import { open } from 'lmdb'; // or require
let myDB = open({
	path: 'my-db',
	// any options go here, we can turn on compression like this:
	compression: true,
});
await myDB.put('greeting', { someText: 'Hello, World!' });
myDB.get('greeting').someText // 'Hello, World!'
// or
myDB.transaction(() => {
	myDB.put('greeting', { someText: 'Hello, World!' });
	myDB.get('greeting').someText // 'Hello, World!'
});

And this happen:


error: Could not resolve: "v8". Maybe you need to "bun install"?

import { setFlagsFromString } from 'v8';
                                   ^
/Users/r/Desktop/projects/a/node_modules/lmdb/node-index.js:5:36 

Bun does not use v8 like deno/node, it uses JavascriptCore from Apple.
I know lmdb officially only supports node and deno. Can we add one for bun?

@kriszyp
Copy link
Owner

kriszyp commented Jan 19, 2023

Yes, I am definitely aware that bun does not use v8, and that is exactly why lmdb-js is careful to only load V8 code if the runtime says it is V8:
https://github.com/kriszyp/lmdb-js/blob/master/node-index.js#L12
However, Bun seems to have become confused lately, in earlier versions it did not report v8, but now Bun has a regression and process.versions on bun reports:

{
  node: "18.13.0",
....
  v8: "10.8.168.20-node.8",
  modules: "108"
}

(note, that "modules" is the specifically the NAN version that interfaces with V8, which Bun also does not support).

This should be filed as a bug with Bun, as it is clearly lying and causing it owns failures in the process.

@rizrmd
Copy link
Author

rizrmd commented Jan 19, 2023

Just posted this issue on bun. I hope it get attention. Anyway, I think bun need to assume v8 to shim some npm modules to make it work. Also there is process.isBun to reliably assume that runtime is bun.

Here is my output for console.log(process):
image

@Jarred-Sumner
Copy link

I think we could remove process.versions.modules, but I'm not sure about process.versions.v8. A decent amount of code expects process.versions.v8 to be a string.

https://sourcegraph.com/search?q=context:global+process.versions.v8.+lang:js&patternType=standard&sm=1&groupBy=repo

https://sourcegraph.com/search?q=context:global+process.versions.modules+lang:js&patternType=standard&sm=1&groupBy=repo

kriszyp added a commit that referenced this issue Jan 20, 2023
@kriszyp
Copy link
Owner

kriszyp commented Jan 21, 2023

@rizrmd v2.7.5 should have support for this latest breaking changes in Bun. Note, that this should enable the standard get/put functionality, but doesn't enable asynchronous transactions. This requires event passing between threads, via the napi_call_threadsafe_function which currently still segfaults in bun, and is reported at oven-sh/bun#158 (comment) (to be fair, I think deno segfaults with that function as well, guess it is hard to implement). I have done a fair bit of work over the last year to try get lmdb-js compatibility in Bun, but asynchronous transactions just won't work without that piece. Anyway, hopefully v2.7.5 will get you going with LMDB on Bun.

@Jarred-Sumner I understand the desire to get this working. I do think we would be better off pushing the ecosystem to adopt better practices (push those that are misusing process.versions.v8 to change rather than those that are correctly using it as detection for V8) instead of just a pursuit of adoption/compatibility with node. This is kinda the same reason we have ridiculous user agent strings in browsers, because browser vendors have placed compatibility/adoption over pushing better practices. And in the examples above, it actually looks like most of the examples in the referenced search really are doing V8 specific actions (that wouldn't make sense in JSC). That being said, this is trivial fix (just using !process.isBun), so certainly not difficult to deal with.

@rizrmd
Copy link
Author

rizrmd commented Jan 24, 2023

Ah, thank you. So I guess this means bun is supported right ?

I will close this issue, and if someone needs async transaction, they should open new issue.

@rizrmd rizrmd closed this as completed Jan 24, 2023
@kriszyp
Copy link
Owner

kriszyp commented Jan 29, 2023

I guess this means bun is supported right ?

I have definitely been trying to maintain support for Bun since it released NAPI support.

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

No branches or pull requests

3 participants