-
Notifications
You must be signed in to change notification settings - Fork 167
docs: add API extractor #354
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
Conversation
Signed-off-by: Grant Timmerman <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
Something is fishy here, but I don't think we removed the exported API. I see some issues that suggest api-extractor has trouble with re-export statements: microsoft/rushstack#663 Maybe we need to just update this line in |
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.
@grant I investigated a bit I think you were having problems because you were adding comments to things we were re-exporting in index.ts
. I think when you do that it clobbers any annotations in the underlying file.
Signed-off-by: Grant Timmerman <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
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.
Thanks @matthewrobertson @anniefu, I updated the generated docs. They should be a bit more comprehensive now too :) PTAL
|
||
"dtsRollup": { | ||
"enabled": true, | ||
"untrimmedFilePath": "docs/generated/api.d.ts", |
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 eventually want to actually make this the types
we use in our package.json
file.
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.
Sure, that's fine. There would need to be some script or tooling to update this generated file with package.json
types.
Or we could simply not generate this file.
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. Please fix the warning before landing.
As a next step it would be good to add an action to automatically validate or generate these when a PR gets submitted.
Signed-off-by: Grant Timmerman <[email protected]>
Add setup for creating API docs for the Node Functions Framework. Does not attempt to annotate whole library (but just enough to make the generator not err).
Fixes #349
Generated API docs: https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/grant_api_extractor/docs/api.md
Note: There aren't that many types right now because we removed a large chunk of the exported API: https://github.com/GoogleCloudPlatform/functions-framework-nodejs/pull/347/files#r724614498
I'm not sure though.
I think we're trying to add that back before releasing a new version of this framework.