search

Level / abstract-leveldown

An abstract prototype matching the leveldown API.
MIT License
146 stars 53 forks source link

src: Allow buffer / nextTick to be overwritten #365

Closed Raynos closed 4 years ago

Raynos commented 4 years ago

There were two dependencies added. Buffer and immediate.

This is for browser support. Instead allow references to Buffer and nextTick to be overwritten by the sub class.

This allows the browser module level-js to import these modules and overwrite the methods using them.

Also in this PR inline xtend and remove it as a dependency.

vweevers commented 4 years ago

How much difference does this make? Most folks use level which includes level-js which will now have to include buffer and immediate itself. So for those folks the difference is 0. Even if you don't use level, but leveldown directly, a single prebuilt binary is bigger than buffer and immediate combined. And if you use level-js in the browser, then its browserify or webpack bundle will now be bigger and have globals that can't easily be excluded.

Raynos commented 4 years ago

It’s not about saving kbs.

It’s about reducing the unique number of npm packages and the unique number of npm authors that have publish rights for those packages.

Each unique npm package is something that has to be audited. I have to potentially fork each module.

I’m curious about the globals that cannot be excluded. Can we make buffer truly optional so that there’s not two copies in level-js ?

As for process.nextTick, maybe the user can configure the bundles to not emulate it ?

That or make nextTick and buffer abstract, aka no defaults

Aka level down and level js have to both overwrite it with their environment implementation defaults, aka node and browser

vweevers commented 4 years ago

It’s about reducing the unique number of npm packages and the unique number of npm authors that have publish rights for those packages.

Gotcha. You'll be glad to know that a while back we removed publish rights for a bunch of inactive folks on level packages, only keeping 2-3 people per package (striking a balance between bus factor and security).

I’m curious about the globals that cannot be excluded. Can we make buffer truly optional so that there’s not two copies in level-js ?

The only way I can think of is excluding it for browsers here, e.g. buffer: false, but that raises new questions, about running browser tests and potential pitfalls of essentially shipping an abstract-leveldown that doesn't work in browsers unless extended (by e.g. level-js).

As for process.nextTick, maybe the user can configure the bundles to not emulate it ?

Latest webpack will do that by default (which triggered this whole discussion). Browserify folks could do the same but it'll break every module that isn't "webpack compatible" yet.

That or make nextTick and buffer abstract, aka no defaults

That just moves the burden to implementors. And some implementations like subleveldown can't make a choice, because they work in both node and browser.

I'm not convinced all this is worth it.

Raynos commented 4 years ago

Yeah I think it’s hard to support both environments.