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

Requirements for manifests #42

Closed
vweevers opened this issue Sep 22, 2018 · 5 comments
Closed

Requirements for manifests #42

vweevers opened this issue Sep 22, 2018 · 5 comments
Labels
discussion Discussion

Comments

@vweevers
Copy link
Member

vweevers commented Sep 22, 2018

I felt the need to defragment various threads:

Prior art:

Requirements:

Open questions:

  • Should manifests extend manifests from underlying downs? Note that underlying downs can be swapped at runtime.
  • In some cases, feature support depends on the runtime environment too. E.g. not all browsers support binary keys in level-js. Is that something we want to expose?
@vweevers
Copy link
Member Author

vweevers commented Sep 8, 2019

Should manifests extend manifests from underlying downs? Note that underlying downs can be swapped at runtime.

Let's take db.clear() as an example, and a few different db's.

level-party db

It may or may not have db.clear() depending on:

  • Whether db is the leader (backed by leveldown) or follower (backed by a multileveldown client)
  • Whether db is open (it's underlying db may be wrapped in deferred-leveldown)
  • Whether db is wrapped in subleveldown (in which case, see below)

In this case, it must be level-party that defines the (hardcoded) manifest, and can include clear in the manifest if all its dependencies support it.

subleveldown db

It may or may not have db.clear() depending on:

  • Whether its input db has clear() (if we refactor subleveldown to unwrap the db early on, in its constructor, then we have the necessary info)
  • Whether it depends on "abstract-leveldown": "^6.0.2" but doesn't implement clear itself and therefor incorrectly deletes all entries of the input db (this is currently the case, and something that would be solved by a manifest)

Here as well, it must be subleveldown that defines the manifest, with the added limitation that the manifest cannot be "static" - as in require('subleveldown').manifest.

@vweevers
Copy link
Member Author

vweevers commented Sep 8, 2019

We can start off simple, with a manifest object that has boolean properties, where each property describes a known feature. We can later handle custom features.

That's a smaller scope than e.g. level-manifest which describes methods of a db and their "type" (readable, writable, sync or async).

In other words, when our manifest lists a feature like { clear: true }) it is assumed to mean that the db has a clear() method with the "standard" function signature.

We can later extend the format into { clear: { whatever } }.

So to be future-proof, I'll rephrase: we can start off with a manifest object has truthy properties.

@vweevers
Copy link
Member Author

vweevers commented Sep 8, 2019

In some cases, feature support depends on the runtime environment too. E.g. not all browsers support binary keys in level-js. Is that something we want to expose?

Yes, and we can. We already feature-detect binary keys so we can just put the result of that in the manifest.

@vweevers
Copy link
Member Author

Ooh. We can also add levelup features like deferredOpen to the manifest. That could make merging levelup and abstract-leveldown smoother. E.g. we can implement deferred open in abstract-leveldown, have it declare supports.deferredOpen = true, then in levelup we'll do:

if (db.supports.deferredOpen) {
  // Use directly
} else {
  // Wrap with deferred-leveldown
}

@vweevers
Copy link
Member Author

Additional requirements can be discussed in https://github.com/Level/supports.

For overall progress, see #83.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion
Projects
None yet
Development

No branches or pull requests

1 participant