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

docs: bump typedoc to 0.26.4 #9727

Merged
merged 2 commits into from
Aug 8, 2024
Merged

docs: bump typedoc to 0.26.4 #9727

merged 2 commits into from
Aug 8, 2024

Conversation

0xpatrickdev
Copy link
Member

@0xpatrickdev 0xpatrickdev commented Jul 17, 2024

closes: #9681
refs: #9729

Description

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

n/a

Testing Considerations

CI and manual QA

Upgrade Considerations

n/a

Copy link

cloudflare-workers-and-pages bot commented Jul 17, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4037399
Status: ✅  Deploy successful!
Preview URL: https://20c3c06a.agoric-sdk.pages.dev
Branch Preview URL: https://pc-bump-typedoc.agoric-sdk.pages.dev

View logs

@dckc
Copy link
Member

dckc commented Jul 17, 2024

no joy: I get a blank page for assertAssetKind

console shows:

Refused to apply style from 'https://pc-bump-typedoc.agoric-sdk.pages.dev/functions/assets/style.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

@0xpatrickdev
Copy link
Member Author

0xpatrickdev commented Jul 17, 2024

no joy: I get a blank page for assertAssetKind

console shows:

Refused to apply style from 'https://pc-bump-typedoc.agoric-sdk.pages.dev/functions/assets/style.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

Let's see if 380d57d does anything. If not, seems we should look into the CDN config

... and strict MIME checking is enabled

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",

@0xpatrickdev 0xpatrickdev force-pushed the pc/bump-typedoc branch 3 times, most recently from 9835a8d to 65bd8d1 Compare July 17, 2024 23:44
@0xpatrickdev
Copy link
Member Author

0xpatrickdev commented Jul 17, 2024

no joy: I get a blank page for assertAssetKind

console shows:

Refused to apply style from 'https://pc-bump-typedoc.agoric-sdk.pages.dev/functions/assets/style.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

This seems to be the culprit: cloudflare/workers-sdk#2240. Updated the PR description to remove the fix for /functions*, we'll tackle that in a separate (forthcoming) ticket.

LuqiPan
LuqiPan previously approved these changes Jul 17, 2024
Copy link
Contributor

@LuqiPan LuqiPan left a 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

@0xpatrickdev
Copy link
Member Author

Now try /funcs instead of /functions: https://pc-bump-typedoc.agoric-sdk.pages.dev/funcs/_agoric_ertp.assertAssetKind

@0xpatrickdev 0xpatrickdev dismissed LuqiPan’s stale review July 18, 2024 02:15

Decided to go for the other bug in this PR

@@ -0,0 +1,137 @@
#!/usr/bin/env node
Copy link
Member

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?

Copy link
Member Author

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!

Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member Author

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

Copy link
Member

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",
Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

See c536276

Copy link
Member Author

@0xpatrickdev 0xpatrickdev Jul 31, 2024

Choose a reason for hiding this comment

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

See c536276

That was naive - I forgot automated testing in this repo doesn't cover this path. Please also see 5cf3e0f which adds required changes to process markdown files.

I tested manually. Unsure if there's a path for testing in agoric/documentation before merging.

@@ -0,0 +1,137 @@
#!/usr/bin/env node
Copy link
Member

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

@turadg turadg mentioned this pull request Jul 31, 2024
@@ -0,0 +1,137 @@
#!/usr/bin/env node
Copy link
Member

Choose a reason for hiding this comment

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

Excellent.

@LuqiPan
Copy link
Contributor

LuqiPan commented Aug 6, 2024

We were talking about dropping our compatibility date on CF to get it working, my test plan is:

  1. drop compatibility date on CF for preview builds
  2. push a commit that removes docs:update-functions-path in docs script in package.json
  3. check if we can see functions with the new build

@LuqiPan
Copy link
Contributor

LuqiPan commented Aug 8, 2024

We were talking about dropping our compatibility date on CF to get it working, my test plan is:
drop compatibility date on CF for preview builds
push a commit that removes docs:update-functions-path in docs script in package.json
check if we can see functions with the new build

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

- with 0.26, typedoc seems to have better support for Functions. currently, web pages crash when these are loaded
@0xpatrickdev 0xpatrickdev added automerge:rebase Automatically rebase updates, then merge and removed automerge:rebase Automatically rebase updates, then merge labels Aug 8, 2024
- avoids limitation in cloudflare pages, where we cannot publish html files to a top-level functions directory
@0xpatrickdev 0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Aug 8, 2024
@mergify mergify bot merged commit e8c4189 into master Aug 8, 2024
88 checks passed
@mergify mergify bot deleted the pc/bump-typedoc branch August 8, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update typedoc to 0.26+
4 participants