Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

feat: split .query into .query and .queryKeys #87

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Apr 15, 2021

The return type of the .query function varies depending on whether
keysOnly is passed as part of the query params. This makes mapping
the return type difficult as it's dependent on the function input.

Here we:

  1. Constrain the return type of .query to just key/value pairs
  2. Add a new .queryKeys method that returns only keys
  3. Add the implementation in the adapter to just chain to the existing .query method and map the result
  4. Remove redundant utils
  5. Remove redundant TextEncoder/Decoder exports
  6. Change the query sort method to be a regular js sort function
  7. Export types for query sort/filter methods

The return type of the `.query` function varies depending on whether
`keysOnly` is passed as part of the query params.  This makes mapping
the return type difficult as it's dependent on the function input.

Here we:

1. Constrain the return type of `.query` to just key/value pairs
1. Add a new `.queryKeys` method that returns only keys
1. Add the implementation in the adapter to just chain to the existing `.query` method and map the result
1. Remove redundant utils
1. Remove redundant TextEncoder/Decoder exports
1. Change the query sort method to be a regular js sort function
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Two nits in the review to consider

}

/**
* @param {KeyQuery} q
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not simply using the Query type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query defines the Filter/Sort functions which use Pair/s as arguments and/or return types. I thought about using generics to define the arg/return types of those methods but it seemed more straightforward to just have another type.

src/tests.js Outdated Show resolved Hide resolved
Co-authored-by: Vasco Santos <vasco.santos@moxy.studio>
achingbrain added a commit to ipfs/js-datastore-core that referenced this pull request Apr 15, 2021
Applies changes from ipfs/interface-datastore/pull/87 and removes
redundant utilities.
achingbrain added a commit to ipfs/js-datastore-level that referenced this pull request Apr 15, 2021
achingbrain added a commit to ipfs/js-datastore-fs that referenced this pull request Apr 15, 2021
achingbrain added a commit to ipfs/js-datastore-s3 that referenced this pull request Apr 15, 2021
@achingbrain achingbrain merged commit 4bb5ebc into master Apr 15, 2021
@achingbrain achingbrain deleted the feat/split-query-into-query-and-query-keys branch April 15, 2021 11:07
achingbrain added a commit to ipfs/js-datastore-core that referenced this pull request Apr 15, 2021
Applies changes from ipfs/interface-datastore/pull/87 and removes
redundant utilities.
achingbrain added a commit to ipfs/js-datastore-level that referenced this pull request Apr 15, 2021
achingbrain added a commit to ipfs/js-datastore-fs that referenced this pull request Apr 15, 2021
jacobheun pushed a commit to ipfs/js-datastore-s3 that referenced this pull request Apr 15, 2021
* feat: split .query into .query and .queryKeys

Applies changes from ipfs/interface-datastore/pull/87

Depends on:

- [ ] ipfs/interface-datastore#87
- [ ] ipfs/js-datastore-core#59

* chore: remove gh urls
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants