-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat!: add getStore
method
#58
Conversation
✅ Deploy Preview for blobs-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
} | ||
``` | ||
|
||
### Environment-based configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this first
src/types.ts
Outdated
export enum HTTPMethod { | ||
Delete = 'delete', | ||
Get = 'get', | ||
Put = 'put', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I bikeshed these to be all caps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keys or the values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excited for this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Like the new API 🚀
@@ -13,73 +13,163 @@ You can install `@netlify/blobs` via npm: | |||
npm install @netlify/blobs | |||
``` | |||
|
|||
### Requirements | |||
|
|||
- Deno 1.30 and above or Node.js 16.0.0 and above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we still have to support node 14? @eduardoboucas
@@ -74,9 +74,7 @@ | |||
"esbuild": "^0.19.0", | |||
"husky": "^8.0.0", | |||
"node-fetch": "^3.3.1", | |||
"p-map": "^6.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay 🥇
src/client.ts
Outdated
// The name of the environment variable that holds the context in a Base64, | ||
// JSON-encoded object. If we ever need to change the encoding or the shape | ||
// of this object, we should bump the version and create a new variable, so | ||
// that the client knows how to consume the data and can advise the user to | ||
// update the client if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: please use JSDoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 99a0e4c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Updates the API with a breaking change, making a
getStore
method the entry point into the library.Before reviewing the actual code, I recommend reviewing the API design itself.
@biilmann your proposed design has been incorporated here.