-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat(vitepress): updates site to use vitepress #971
Conversation
to do: (!) Found dead link /guides/agoric-cli/README in file guides/getting-started/deploying.md (!) Found dead link /ertp/api/index in file guides/ertp/issuers-and-mints.md (!) Found dead link http://localhost:3000 in file guides/dapps/index.md (!) Found dead link /guides/wallet/README in file guides/coreeval/permissions.md (!) Found dead link /ertp/api/index in file guides/ertp/purses-and-payments.md (!) Found dead link ./../../glossary/README in file guides/js-programming/eventual-send.md (!) Found dead link ./../zoe/actual-contracts/index in file guides/integration/name-services.md (!) Found dead link ./../zoe/actual-contracts/index in file guides/integration/name-services.md (x2) (!) Found dead link /repl/timerServices in file guides/js-programming/notifiers.md (!) Found dead link ./../../glossary/README in file guides/js-programming/notifiers.md (!) Found dead link /zoe/guide/contracts/index in file guides/zoe/index.md (!) Found dead link /zoe/guide/contracts/constantProductAMM in file guides/zoe/index.md + external redirects + subpath redirects
Deploying documentation with Cloudflare Pages
|
<template> | ||
<div class="zoe-version"> | ||
Zoe {{ zoeVersion }}. Last updated | ||
{{ zoeDocsUpdated }}. |
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.
For the future, do we we get value out of this? It's hardcoded to August 25, 2022
. VuePress had a lastUpdatedAt
plugin for individual .md
's that is also available in VitePress (and active on our site)
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 it's supposed to indicate when the zoe code was last updated. It might be nice to say what version of code we're documenting.
But yeah... currently, this is just noise.
cd45bad
to
de16bdb
Compare
de16bdb
to
c283f04
Compare
Deployed something here for the interim: https://65cac5889558bd35423c70e1--beautiful-lolly-46c6b2.netlify.app/ I used the output of |
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.
found a couple important things
"docs:build-cf": "NODE_OPTIONS=--openssl-legacy-provider vuepress build main && cp _redirects main/.vuepress/dist/", | ||
"docs:build-root": "NODE_OPTIONS=--openssl-legacy-provider yarn docs:build && yarn docs:re-root && cp _redirects main/.vuepress/dist-root/", | ||
"docs:re-root": "cd main && mkdir -p .vuepress/dist-root/ && cp -rp .vuepress/dist/ .vuepress/dist-root/documentation/", | ||
"check-links": "NODE_OPTIONS=--openssl-legacy-provider vuepress check-md main", |
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.
what happened to check-links
?
important
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.
check-links
is noted in CONTRIBUTING.md
. I wonder if this PR is consistent with the rest of the stuff in there.
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.
check-links
logic is still there - it's natively part of of vitepress via ignoreDeadLinks, configured here, and will run during yarn docs:build
.
Example failure:
yarn docs:build
yarn run v1.22.5
$ NODE_OPTIONS=--openssl-legacy-provider vitepress build main
vitepress v1.0.0-rc.42
(!) Found dead link ./../js-fprogramming/index in file guides/zoe/contract-upgrade.md
If this is expected, you can disable this check via config. Refer: https://vitepress.dev/reference/site-config#ignoredeadlinks
✖ building client + server bundles...
build error:
Error: [vitepress] 1 dead link(s) found.
CONTRIBUTING.md changes are on the radar, added two more commits 6320811, 5594baa that make adjustments.
- simplifies CI, which currently looks in main/.vitepress/dist or main/.vuepress/dist
objects that last across upgrades ([durable objects](./contract-upgrade.md#durability)) later. | ||
objects that last across upgrades ([durable objects](./contract-upgrade#durability)) later. |
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.
odd... links don't go to xyz.md
anymore?
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.
They do, but the the extensions are now optional so I chose to remove them for simplicity.
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.
Hm. simplicity vs. consistency...
CONTRIBUTING still says
Next, your Markdown links should be to the
.md
Markdown files ...
So unless we aim to change the shop norm, I prefer consistency. (not critical)
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.
The wording of that sentence is a bit confusing to me. Updated here 37b292d
main/.vitepress/enhanceApp.js
Outdated
// XXX | ||
// vitepress does not seem to expose the vue router directly, so we'll | ||
// have to refactor this. some redirects are working in themeConfig/rewrites.js | ||
|
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.
@gibson042 when you have some time, could you take a look at this PR and help me understand #882 and #888 better?
For #888, external redirects, is it sufficient to use _redirects
and handle this outside the application? Cloudflare Pages Redirects
For #882, I'm not sure I understand the motivation enough to tell if the issue is still persistent with vitepress.
If you are looking for hosted preview URL, try this: https://0ebcfb0d.documentation-7tp.pages.dev/
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.
Let's check #888 first:
- https://0ebcfb0d.documentation-7tp.pages.dev/guides/js-programming/ses/lockdown.html is good ✔
- https://0ebcfb0d.documentation-7tp.pages.dev/zoe/guide/contracts/covered-call.html is not ✘
I think the latter can be fixed with greedy matching, e.g.
/zoe/guide /guides/zoe/
/zoe/guide/* /guides/zoe/:splat
So probably we just want to map redirects
from main/.vuepress/enhanceApp.js into _redirects:
- Each prefix-style entry in the former should become two entries in the latter as above.
- Each leaf-page entry in the former should become two entries in the latter to the same destination, one with a source ending in ".html" and the other not (e.g. https://0ebcfb0d.documentation-7tp.pages.dev/wallet-api.html and https://0ebcfb0d.documentation-7tp.pages.dev/wallet-api should both work).
- For proper behavior, at least all non-external redirects should use status code 301 (Moved Permanently).
As for #882, it looks like we do need to carry the kludge forward (or find a way to fix things upstream). Observe how clicking either of the following links in the foreground (e.g., not a background tab) scrolls away from the in-page anchor to the top of page:
- https://0ebcfb0d.documentation-7tp.pages.dev/reference/zoe-api/zoe-data-types.html#keywordrecord
- https://0ebcfb0d.documentation-7tp.pages.dev/reference/zoe-api/zoe-data-types.html#amountkeywordrecord
But something is different, because even manual application doesn't appear to be correcting the issue.
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 @gibson042 !
For #888, it turns out the build you were looking at already had the custom scroll logic in place. In 5983738 I removed the logic, and scroll to anchor on initial page load seems to work fine (as a nit, it could scroll up a bit more, but it seems to be fine from my POV)
For #882 (redirects), I followed your suggestion in c69ed8a:
I am not sure if we need something like: /ertp/guide/ /guides/ertp/
in addition to /ertp/guide /guides/ertp/
, but it doesn't seem to cause any harm. /* /:splat
seems to cover the covered-call example you shared. Verified on this deploy url.
Edit:
Tested these links:
https://65f33b9537ffb20c818e3c7a--visionary-marzipan-3cd6de.netlify.app/reference/zoe-api/zoe-data-types#keywordrecord
https://65f33b9537ffb20c818e3c7a--visionary-marzipan-3cd6de.netlify.app/reference/zoe-api/zoe-data-types.html#keywordrecord
It does not seem we have an #amountkeywordrecord
anchor on the page, so excluding that from my test cases.
https://65f33b9537ffb20c818e3c7a--visionary-marzipan-3cd6de.netlify.app/zoe/guide/contracts/covered-call
https://65f33b9537ffb20c818e3c7a--visionary-marzipan-3cd6de.netlify.app/zoe/guide/contracts/covered-call.html
- removes code added in #882 that aimed to fix anchor tag linking on initial page loads as it no longer seems necessary with vitepress
- restores current styles on docs.agoric.com, ensuring users do not need to horizontally scroll
- ports vuepress redirects (client side) to cloudflare _redirects - see comment: #971 (comment)
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.
Let's go for it.
I'm somewhat unsure that I reviewed this closely enough to find all possible regressions, but the benefit of fulltext search and a maintained platform are sufficient that it's worth the risk. If we find regressions, we can address them in due course.
To merge this, we need to change the Build Output Directory in the cloudflare dash from We could make this deploy from |
- there are probably add'l duplicates that can be removed - seems the /* /:splat pattern should cover many of individual page links
cc37f6d
to
b1eed18
Compare
closes: #972
refs: #609
Summary
To Do
_redirects_
)ignoreDeadLinks: true
Think there's room for improvements here?Preserving existing styles for now: 4bcb1ae,mlc-config.json
enhanceApp.js
I would like to to understand feat: Improve the jump-to-fragment fixup for non-heading targets #882 and fix: Handle redirects as prefixes #888 better to see if we can removeenhanceApp.js
..vitepress/config.mjs
(formerly,.vuepress/config.js
) see 5983738