dinero.js
dinero.js copied to clipboard
docs: adding api-extractor
adding @microsoft/api-extractor will allow for rolling up declaration files, tracking api changes, and generating api documentation.
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/dinerojs/dinerojs/2fuXbAX3EQ88xA377mYVNYf4P2kP
✅ Preview: https://dinerojs-git-fork-johnhooks-add-api-extractor-dinerojs.vercel.app
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 814c449a1b5fd0c0597024a0c8b1247e2c2ef6ec:
Sandbox | Source |
---|---|
@dinero.js/example-cart-react | Configuration |
@dinero.js/example-cart-vue | Configuration |
@dinero.js/example-pricing-react | Configuration |
@dinero.js/example-starter | Configuration |
What's this part for? If we can have a JSON file instead (I think it's possible) which we can then leverage in the site with MDX, it's better.
@sarahdayan, it does generate a JSON file but it is pretty complex, it can be used by the document generator to create a set of linked markdown files. Though it could be read with the api-extractor-model package and used how ever you want. Without any extra config the markdown output looks like this example.
By the way, all the builds failed because I didn't update the lock file. I'll do that in future commits, but I assume that file will be tricky and probably need to be generated after an upstream merge, right before this PR is accepted.
I have added api-extractor
to core, the most interesting for review is the packages/core/etc/core.api.md
file. Its for tracking api changes, alert for errors, and show what is public but isn't documented.
I haven't added a script to run api-extractor
yet, but if you want to play with it, you can run it with the following. Right now it has a bunch of warning, but most are about needing to add the @public
flag to all the comments of the public facing api.
-
cd packages/core
-
npx tsc -p .
-
npx api-extractor run --local --verbose
This also generates a rolled up declaration file under packages/core/dist/code.d.ts
.
Would all of core be considered @internal?
Thanks a lot for PRing @johnhooks. I'm unable to look into it for the moment but I'll have more time next week.
Totally understand. I started trying to use it in some private projects and it's been educational. I'll try to push a new commit that implements some of what I've learned. Also I will start with getting it working in the dinero.js
package. It's the most public facing package and make the most sense for implementation.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated |
---|---|---|---|
dinerojs | ✅ Ready (Inspect) | Visit Preview | Nov 6, 2022 at 4:18PM (UTC) |
@sarahdayan I modified packages/dinero.js
to use api-extractor. Now that package's scriptbuild:types
generates a single type file into dinero.js/packages/dinero.js/dist/dinero.js.d.ts
. I did need to make a few changes to incorporate api-extractor, some config files, source comments, and some modifications to the tsconfig.json
in the package folder. Api-extractor uses the location of the tsconfig.json
to discover a projects location. I am sure I could override the defaults to use the tsconfig.declaration.json
, if that is important. Also, I totally understand this is a super low priority. I restarted my investigation of this as a way to learn how to merge a fork with an upstream repo.
I added api-extractor
to @dinero.js/currencies
. Are all the currencies generated by a script or written by hand? To quiet api-extractor
warnings about commenting which exports are public I need to add a @public
comment to all the currencies.
~~@sarahdayan over all, my feelings about this solution is that it's complex and api-extractor
is built to be incorporated into monorepos managed by rush
. I'm going to look at https://github.com/wessberg/rollup-plugin-ts, and see if maybe it's a better solution to rollup the declarations, without all the overhead and config of this PR.~~
EDIT: Maybe I'm being to critical. It would be very handy to author most of the documentation in the source comments, and be able to pull that and the type declaration into a mdx file and then add additional resources and examples in that file.
@sarahdayan when I rebased into main, it changed my main branch, not the add-api-extractor
branch. Sorry I'm very new to the PR/git process.
@johnhooks No worries, Git is hard.
Here's the process:
- Remove your local
main
branch (git branch -d main
) - Checkout
main
(git checkout main
) - Pull the latest changes (
git pull -r
) - Checkout your branch (
git checkout add-api-extractor
) - Rebase onto
main
(git rebase main
) - Force push your branch (
git push add-api-extractor --force
)
@sarahdayan is that supposed to take me though a long interactive process for each commit? Just making sure I'm doing this right.
@johnhooks If there are conflicts, yes. You can also merge main
, it should be easier than rebasing.
@sarahdayan I already merge upstream in https://github.com/dinerojs/dinero.js/pull/499/commits/0b541b02b0fa314b69d8203e3f9b010357f6f8e6
@johnhooks That was before I made the change, so you need to do it again.
@sarahdayan okay, I'll keep slogging through this. One issue I'm having, vscode isn't allows suggesting to open the merge editor. whats the right cli way to resolve merge conflicts
@johnhooks I just use Vi, but I think you can set things up to use another editor.
If you want for this time I can handle the merge?
@sarahdayan I think I did it!
@sarahdayan awesome! This is my first big commit to an open source repo, I'm pumped!
@sarahdayan I think some where in the rebase to make things work I delete the yarn.lock file! I though it was going to keep the original and just get rid of mine, but I don't see it in the repo. Sorry
@johnhooks Ah, indeed, I missed that during my last review. I'll re-generate it.
I was just working to try to do that, just checking so I make sure I know how, I would checkout the lock file from the commit right before mine and then run yarn install, add the file and commit?
@johnhooks You'd need to go back to the previous commit, copy the file, get to the head, paste the file, then run yarn install
.
I re-created the file so no worries.