Skip to content

fix: update TypeScript declaration for KV#1608

Closed
raymondfeng wants to merge 1 commit intomasterfrom
kv-types
Closed

fix: update TypeScript declaration for KV#1608
raymondfeng wants to merge 1 commit intomasterfrom
kv-types

Conversation

@raymondfeng
Copy link
Contributor

@raymondfeng raymondfeng commented Jul 13, 2018

Description

update TypeScript declaration for KV

Related issues

  • connect to <link_to_referenced_issue>

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

static iterateKeys(
filter?: Filter,
options?: Options,
): Iterator<Promise<string>>;
Copy link
Contributor Author

@raymondfeng raymondfeng Jul 13, 2018

Choose a reason for hiding this comment

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

This is not accurate. It should be AsyncIterator<string> which is only available with esnext. See https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-3.html.

Caveats

  • Keep in mind that our support for async iterators relies on support for Symbol.asyncIterator to exist at runtime.
  • You may need to polyfill Symbol.asyncIterator, which for simple purposes can be as simple as: (Symbol as any).asyncIterator = Symbol.asyncIterator || Symbol.for("Symbol.asyncIterator");
  • You also need to include esnext in your --lib option, to get the AsyncIterator declaration if you do not already have it.
  • Finally, if your target is ES5 or ES3, you’ll also need to set the --downlevelIterators flag.

* @promise
*/
static deleteAll(
key: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think it's supposed to take a key argument for deleteAll.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Will fix.

@raymondfeng raymondfeng force-pushed the kv-types branch 5 times, most recently from ade9f0f to 346d18b Compare July 16, 2018 16:22
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

LGTM with one important comment.

I wish iterateKeys was conforming to AsyncIterator type from TypeScript and ES2019, but that's out of scope of this patch.

*/
keys(
static keys(
filter?: Filter,
Copy link
Member

Choose a reason for hiding this comment

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

KVFilter, not Filter.

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

Successfully merging this pull request may close these issues.

3 participants