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

Increase API parity with levelup #383

Closed
wants to merge 22 commits into from
Closed

Increase API parity with levelup #383

wants to merge 22 commits into from

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Oct 3, 2021

Biting the bullet. This adds promise support, makes open() & close() idempotent, adds open and close events, makes an abstract-leveldown db safer than a levelup db, and more.

Needs a ton of (canary) tests. Contains a few subtle breaking changes, but for consumers of downstream implementations it should be painless. An abstract-leveldown db can still be wrapped with levelup and folks can continue to use that interface.

I'm opening this up for early review, I might take it even further.

Ref Level/community#58
Ref Level/leveldown#8

Non-breaking because these were undocumented and temporary options
for internal `levelup` compatibility tests.
Very similar to `levelup` but more precise. If `open()` and
`close()` are called repeatedly (while the previous call has not
yet completed) the last call dictates the final status. Callbacks
are not called until any pending state changes are done, meaning
that the status is not 'opening' or 'closing'. Same for events.

For example, in a sequence of calls like `open(); close(); open()`
the final status will be 'open', only the second call will error,
only an 'open' event is emitted, and all callbacks will see that
status is 'open'. The callbacks are called in the order that the
`open()` or `close()` calls were made.

In addition, unlike on `levelup`, it is safe to call `open()` while
status is 'closing'. It will wait for closing to complete and then
reopen.

We should now have complete safety, including in `leveldown` because
the native code there delays `close()` if any operations are in
flight. In other words, the JavaScript side in `abstract-leveldown`
prevents new operations before opening the db, and the C++ side in
`leveldown` prevents closing the db before operations completed.

Ref Level/leveldown#8
Ref Level/community#58
@vweevers vweevers added the semver-major Changes that break backward compatibility label Oct 3, 2021
@vweevers
Copy link
Member Author

vweevers commented Oct 9, 2021

Complexity is slowly increasing here and I'm wary of that. So far it's still worth it. I find the behavior easier to oversee, and reimplementing certain levelup features (like idempotent open) gives me a chance to polish them and remove the need for safety checks that we've had to add to leveldown.

I've been canary testing this against various modules; subleveldown in particular because I sense that this module will tell me when I've found a balance between increased abstract-leveldown complexity and reduced subleveldown complexity.

@vweevers
Copy link
Member Author

Funny how this is starting to look like the levelup from 8 years ago.

An abstract-leveldown db can still be wrapped with levelup and folks can continue to use that interface.

Technically that still works (with a few tweaks in levelup) but there's no value in doing that anymore (except if you need streams, but that will be easy to move to a module). Thinking about forking abstract-leveldown including dependents, for a clean break. For now I'm continuing here, because I have no reason to publish it yet and I've yet to prove this is all worth it. Next up are encodings and maybe a light form of hooks.

With tests imported from `levelup`, `encoding-down` and `memdown`.

Has a temporary github dependency: `Level/transcoder`.
For lack of TextEncoder.
@vweevers
Copy link
Member Author

vweevers commented Nov 9, 2021

@vweevers vweevers closed this Nov 9, 2021
@vweevers vweevers deleted the api-parity branch November 9, 2021 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major Changes that break backward compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant