Skip to content
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

Small fixes and setup API extractor #6

Merged
merged 12 commits into from
Dec 12, 2023
Merged

Small fixes and setup API extractor #6

merged 12 commits into from
Dec 12, 2023

Conversation

chgeo
Copy link
Member

@chgeo chgeo commented Dec 11, 2023

Set up API extractor → run with npm run api-extractor

Add a few fixes to allow it tun run (see below)

Fix a bunch of TSDoc comments (@example, code fences, @param)

apis/cds.d.ts Show resolved Hide resolved
apis/server.d.ts Outdated Show resolved Hide resolved
apis/server.d.ts Show resolved Hide resolved
@chgeo chgeo requested a review from daogrady December 11, 2023 16:11
apis/linked.d.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
chgeo and others added 2 commits December 12, 2023 10:06
Co-authored-by: Daniel O'Grady <103028279+daogrady@users.noreply.github.com>
@daogrady
Copy link
Contributor

daogrady commented Dec 12, 2023

Running npm run api-extractor greets me with the following output:

➜  cds-types git:(api-extractor) ✗ npm run api-extractor

> @cap-js/cds-types@0.1.0 api-extractor
> api-extractor run --local --verbose


api-extractor 7.38.5  - https://api-extractor.com/

Using configuration from ./api-extractor.json
Analysis will use the bundled TypeScript version 5.0.4
*** The target project appears to use TypeScript 5.3.3 which is newer than the bundled compiler engine; consider upgrading API Extractor.
Writing: /Users/-/git/cds-types/temp/cds-types.api.json
Error: Unable to create the API report file. Please make sure the target folder exists:
/Users/-/git/cds-types/etc
Writing package typings: /Users/-/git/cds-types/dist/cds-types.d.ts
Warning: apis/cqn.d.ts:59:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:61:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:63:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:65:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:69:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:73:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:75:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:77:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:79:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:81:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration
Warning: apis/cqn.d.ts:83:5 - (tsdoc-undefined-tag) The TSDoc tag "@private" is not defined in this configuration

The warnings can surely be fixed later, but we should probably look into the error.
Creating the missing folder now produces the following output:

➜  cds-types git:(api-extractor) ✗ npm run api-extractor

> @cap-js/cds-types@0.1.0 api-extractor
> api-extractor run --local --verbose


api-extractor 7.38.5  - https://api-extractor.com/

Using configuration from ./api-extractor.json
Analysis will use the bundled TypeScript version 5.0.4
*** The target project appears to use TypeScript 5.3.3 which is newer than the bundled compiler engine; consider upgrading API Extractor.
Writing: /Users/-/git/cds-types/temp/cds-types.api.json
Warning: The API report file was missing, so a new file was created. Please add this file to Git:
/Users/-/git/cds-types/etc/cds-types.api.md

Ergo:

  • should we add ./etc/ to the repository? Or prefix the api-extractor script with mkdir etc && ...? The latter will probably cause issues on Windows.
  • I did not find any entry in the config, so I assume "etc" is the default name. If that is not the case and we have no other point for "etc", can we find a more fitting name? "extracted-api" or similar?
  • should we follow the tool's recommendation and add etc/cds-types.api.md to the repository?
  • (probably beyond the scope of this PR, but as api-extractor hints at it, we might want to upgrade TS to 5.3.3)

@chgeo
Copy link
Member Author

chgeo commented Dec 12, 2023

  • should we add ./etc/ to the repository? Or prefix the api-extractor script with mkdir etc && ...? The latter will probably cause issues on Windows.
  • I did not find any entry in the config, so I assume "etc" is the default name. If that is not the case and we have no other point for "etc", can we find a more fitting name? "extracted-api" or similar?
  • should we follow the tool's recommendation and add etc/cds-types.api.md to the repository?
  • (probably beyond the scope of this PR, but as api-extractor hints at it, we might want to upgrade TS to 5.3.3)

All valid points, but for a second PR.
My goal here was not to establish the extractor, but to fix some findings.

@chgeo chgeo enabled auto-merge (squash) December 12, 2023 11:10
Copy link
Contributor

@daogrady daogrady left a comment

Choose a reason for hiding this comment

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

Then let's postpone the proper setup of api-extractor to another PR and get these declarations merged.
Looks good to me now, thanks for brushing up the types and docs.

@chgeo chgeo merged commit 27bb0ef into main Dec 12, 2023
3 checks passed
@chgeo chgeo deleted the api-extractor branch December 12, 2023 11:10
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.

2 participants