-
Notifications
You must be signed in to change notification settings - Fork 212
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
docs: bump typedoc to 0.26.4 #9727
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
no joy: I get a blank page for assertAssetKind console shows:
|
Let's see if 380d57d does anything. If not, seems we should look into the CDN config
No luck, removed. Here was the diff diff --git a/docs/_headers b/docs/_headers
new file mode 100644
index 0000000000..9b87c50354
--- /dev/null
+++ b/docs/_headers
@@ -0,0 +1,6 @@
+/*.css
+ Content-Type: text/css
+/*.js
+ Content-Type: text/javascript
+/*.html
+ Content-Type: text/html
diff --git a/package.json b/package.json
index 271c1eac51..f04ba33721 100644
--- a/package.json
+++ b/package.json
@@ -48,7 +48,7 @@
"scripts": {
"clean": "yarn lerna run --no-bail clean",
"check-dependencies": "node ./scripts/check-mismatched-dependencies.cjs",
- "docs": "typedoc --tsconfig tsconfig.build.json",
+ "docs": "typedoc --tsconfig tsconfig.build.json && cp docs/_headers api-docs/_headers",
"docs:markdown-for-agoric-documentation-repo": "typedoc --plugin typedoc-plugin-markdown --tsconfig tsconfig.build.json",
"lerna": "lerna",
"link-cli": "yarn run create-agoric-cli", |
9835a8d
to
65bd8d1
Compare
This seems to be the culprit: cloudflare/workers-sdk#2240. Updated the PR description to remove the fix for |
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.
🚢
Thank you for looking into this and getting it upgraded
38c9422
to
bf1c348
Compare
Now try |
Decided to go for the other bug in this PR
@@ -0,0 +1,137 @@ | |||
#!/usr/bin/env node |
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.
did you write this whole script today?
why .cjs rather than .mjs?
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.
Yes - I paired with an LLM to to help arrive at the solution more quickly. This sort of task seemed like the perfect candidate.
And I instinctively went for .cjs as this is a build script. Will note for the future .mjs works and is preferred!
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'm skeptical of the amount of code here to work around the CF bug. Did TypeStrong/typedoc#2111 (comment) not work?
TypeStrong/typedoc#2111 (comment) is more work but looks soild.
I will approve this because it fixes the immediate problem:
https://agoric-sdk.pages.dev/funcs/_agoric_orchestration.buildRootObject
https://pc-bump-typedoc.agoric-sdk.pages.dev/funcs/_agoric_orchestration.buildRootObject
but please file an issue to come back and remove this file. I'd suggest to just merge this now but it's going to need a rebase anyway so please include in a doc comment why this file is needed, what alternatives were researched and what are possible next steps
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.
Did TypeStrong/typedoc#2111 (comment) not work
It does not since we have JS enabled. Part of the script reaches into a build-generated base64 string in a js file that contains navigation state to also update the urls there.
TypeStrong/typedoc#2111 (comment) is more work but looks soild
I didn't try this one - the git links are dead and it seems like PlayForm/Build
might do something different now - I don't see any mentions of a typedoc theme in the README. This also involves an extra dependency.
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.
but please file an issue to come back and remove this file. I'd suggest to just merge this now but it's going to need a rebase anyway so please include in a doc comment why this file is needed, what alternatives were researched and what are possible next steps
We can leave #9729 open for this - i've updated to refs. PTAL at 092acb7 for an updated file description
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.
Excellent.
"docs:markdown-for-agoric-documentation-repo": "typedoc --plugin typedoc-plugin-markdown --tsconfig tsconfig.build.json", | ||
"docs:update-functions-path": "node ./scripts/update-typedoc-functions-path.cjs", |
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.
confirming update-functions-path
isn't needed for markdown-for-agoric-documentation-repo
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'm not certain here - are we currently using docs:markdown-for-agoric-documentation-repo
? If yes, it seems this should be updated.
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.
See c536276
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.
@@ -0,0 +1,137 @@ | |||
#!/usr/bin/env node |
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'm skeptical of the amount of code here to work around the CF bug. Did TypeStrong/typedoc#2111 (comment) not work?
TypeStrong/typedoc#2111 (comment) is more work but looks soild.
I will approve this because it fixes the immediate problem:
https://agoric-sdk.pages.dev/funcs/_agoric_orchestration.buildRootObject
https://pc-bump-typedoc.agoric-sdk.pages.dev/funcs/_agoric_orchestration.buildRootObject
but please file an issue to come back and remove this file. I'd suggest to just merge this now but it's going to need a rebase anyway so please include in a doc comment why this file is needed, what alternatives were researched and what are possible next steps
bf1c348
to
c536276
Compare
@@ -0,0 +1,137 @@ | |||
#!/usr/bin/env node |
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.
Excellent.
We were talking about dropping our compatibility date on CF to get it working, my test plan is:
|
No dice 😢 I dropped compatibility date on both CF for preview builds and in the commit via wrangler.toml Then I pushed a commit that removes docs:update-functions-path in docs script in package.json However, the functions are still not showing: https://d52c115a.agoric-sdk.pages.dev/funcs/_agoric_ertp.assertAssetKind |
94dd32d
to
5cf3e0f
Compare
- with 0.26, typedoc seems to have better support for Functions. currently, web pages crash when these are loaded
5cf3e0f
to
ce9a133
Compare
- avoids limitation in cloudflare pages, where we cannot publish html files to a top-level functions directory
ce9a133
to
4037399
Compare
closes: #9681
refs: #9729
Description
/functions
path to/funcs
to avoid 🐛 BUG: Static page with a category named 'functions' (public/functions/index.html) gets discarded cloudflare/workers-sdk#2240Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
CI and manual QA
Upgrade Considerations
n/a