-
Notifications
You must be signed in to change notification settings - Fork 16
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!: export v2 api #408
feat!: export v2 api #408
Conversation
For testing, I published this as |
package.json
Outdated
@@ -52,6 +52,7 @@ | |||
"test": "test" | |||
}, | |||
"dependencies": { | |||
"@netlify/serverless-functions-api": "^1.5.1", |
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 wonder if we should lock the version, such that the entire API surface in one version of @netlify/functions
is immutable? Probably not a huge deal since we're also the authors of the dependency and we can control the releases we make, but wondering if it's a good caution to put in place.
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.
Good idea 👍
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 423960c
src/function/index.ts
Outdated
@@ -2,3 +2,4 @@ export { Context as HandlerContext } from './context.js' | |||
export { Event as HandlerEvent } from './event.js' | |||
export { BuilderHandler, Handler, BackgroundHandler, HandlerCallback, StreamingHandler } from './handler.js' | |||
export { BuilderResponse, Response as HandlerResponse, StreamingResponse } from './response.js' | |||
export * from './v2.js' |
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.
Any reason this isn't a named export?
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.
No particular reason, no. I put the named exports inside of v2.js
, but I can also move that up if you think that's better.
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.
Fine to re-export, but I think it's better to do an explicit named export in both cases if you do (and make it export type ...
)
I'll hold off on merging this until we want to publish it. For testing, we can use the prerelease. |
The change looks good @eduardoboucas. I see we have a 2nd interface in this repo called functions/src/function/index.ts Line 1 in 4f4304e
|
🤖 I have created a release *beep* *boop* --- ## [2.0.0](v1.6.0...v2.0.0) (2023-08-22) ### ⚠ BREAKING CHANGES * export v2 api ([#408](#408)) ### Features * export v2 api ([#408](#408)) ([894fb91](894fb91)) ### Bug Fixes * **deps:** update dependency @netlify/serverless-functions-api to v1.7.0 ([#414](#414)) ([6fba3af](6fba3af)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
This PR adds the V2 functions API to the exports of
@netlify/functions
.