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: MDX plugin to interpret link target URLs based on source file directory #1170
Conversation
…injected via options
| const visitPromise = import("unist-util-visit"); | ||
| const expandVersionedUrl = require("./expandVersionedUrl"); | ||
|
|
||
| const resolve = async () => { | ||
| visit = (await visitPromise).visit; | ||
| }; | ||
|
|
||
| resolve(); |
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.
This import/resolve nonsense irritates me, but I was led here due to the following:
requireis not supported. And in fact it suggest I use a dynamicimport()instead:
[ERROR] Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/stevenhicks/sjh/dev/camunda/camunda-platform-docs/node_modules/unist-util-visit/index.js from /Users/stevenhicks/sjh/dev/camunda/camunda-platform-docs/src/mdx/versionedLinks.js not supported.
Instead change the require of index.js in /Users/stevenhicks/sjh/dev/camunda/camunda-platform-docs/src/mdx/versionedLinks.js to a dynamic import() which is available in all CommonJS modules.
I don't know enough about modules to know why this is, or if there's a way to make it accept require(...).
await import()is not supported, no top-level await:
[ERROR] /Users/stevenhicks/sjh/dev/camunda/camunda-platform-docs/src/mdx/versionedLinks.js:1
const visit = await import("unist-util-visit");
^^^^^
SyntaxError: await is only valid in async functions and the top level bodies of modules
I thought that maybe I could rename from .js to .mjs files to signal that they were modules, but that didn't work.
I didn't want to spend a lot of time dealing with build issues, so I just accepted this code.
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.
Huh, is this d-🦖?
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 so, but what's confusing is that the docs for writing custom MDX plugins in docusaurus use require. I'm planning to reach out via discord to see if I'm the only one experiencing this.
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.
Apparently there is a disclaimer in the docs that is intended to explain this:
NOTE
There's recently a trend in the Remark/Rehype ecosystem to migrate to ES Modules, a new JavaScript module system, which Docusaurus doesn't support yet. Please make sure your installed plugin version is CommonJS-compatible before we officially support ESM. Alternatively, you can read about using dynamic import() as a workaround in the tutorial of installing rehype-katex.
If we want to use require instead of dynamic imports, we need to install an older version of unist-util-visit:
You need to use unist-util-visit v2
I'm personally fine with this clunky import/await code so I don't intend on rewriting with that version.
| @@ -0,0 +1,43 @@ | |||
| const tokens = [ | |||
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.
These tokens and rules are defined in a way that I think could come from configuration instead, in case we decide to share this plugin with the rest of the world. I'm intending on reaching out to the docusaurus community to see if this kind of thing is useful for others.
|
|
||
| const versionedLinks = (options) => { | ||
| const transformer = async (ast, vfile) => { | ||
| await visit(ast, "link", (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.
This visits every single markdown link in the docs. I think I've provided enough early-exits in the expandVersionedUrl function that we don't have to worry about builds slowing down....that also might even be premature optimization on my part. But if we do notice build times go way up, there might be some micro-optimizations we can make here?
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.
We would see this on local builds too right? I may be the only person who builds locally anymore, but I tend to do that when I'm making significant link changes, particularly across multiple pages.
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, if there's a performance degradation we'd see it on local builds too.
For what it's worth, and anecdotally, developing locally with npm start doesn't appear any slower than it used to.
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.
Big proponent of building locally before pushing anything @akeller 🙏
| @@ -29,7 +31,8 @@ | |||
| "mixpanel-browser": "^2.45.0", | |||
| "react": "^17.0.2", | |||
| "react-dom": "^17.0.2", | |||
| "react-player": "^2.10.1" | |||
| "react-player": "^2.10.1", | |||
| "unist-util-visit": "^4.1.0" | |||
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.
WAT!! I remember when I had to do this in vanilla JS to parse JSON objects.
| { | ||
| token: "$docs$", | ||
| rules: [ | ||
| // these mappings are currently wrong because there are no optimize versions yet...but eventually, this is what we'll want. | ||
| { match: "/optimize/", expandTo: "/docs/next" }, | ||
| { match: "/optimize_versioned_docs/version-3.8.0/", expandTo: "/docs" }, | ||
| { | ||
| match: "/optimize_versioned_docs/version-3.7.0/", | ||
| expandTo: "/docs/1.3", | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| token: "$optimize$", | ||
| rules: [ | ||
| // these mappings are currently wrong because there are no optimize versions yet...but eventually, this is what we'll want. | ||
| { match: "/docs/", expandTo: "/optimize/next" }, | ||
| { match: "/versioned_docs/version-8.0/", expandTo: "/optimize" }, | ||
| { match: "/versioned_docs/version-1.3/", expandTo: "/optimize/3.7.0" }, | ||
| ], | ||
| }, | ||
| ]; |
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.
Thinking ahead here, would we (DX) then add (and adjust) the rules for 8.1 or would we ask the Optimize team to do this? Whose responsibility is it? Feels like DX if we are the ones publishing the release.
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 would anticipate DX owning this, but that's because I'm assuming we're the ones who cut new versions of the main docs when they're released, too. We'd have to run docusaurus version to generate the new version number in addition to changing these rules, and that feels like infrastructure work for us.
Right?
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 are on the same page here.
🔮 I foresee us updating the docs-on-docs to include clear responsibilities around these minor product releases vs. continuous product releases (when we need to run docusaurus version vs. just releasing). These should probably have a clear name too.
| describe("expandVersionedUrl", () => { | ||
| describe("unexpandable URLs", () => { | ||
| const sourcePath = | ||
| "/Users/monkeypants/camunda-platform-docs/optimize/what-is-optimize.md"; |
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.
My favorite username 🐒 👖
|
@pepopowitz this would require a 1 time update to all internal links in the docs? Or just all internal Optimize links? |
@akeller it would be a 1 time update to:
I would consider that all to be part of #1116. |
@akeller based on the way those buttons span across left-sidebar sections, my assumption is they're based on the sidebars files. This is again covered by #1116, which includes synchronizing the sidebars files for main docs with optimize, basically to preserve this (and the left nav) structure. |
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.
No comments on my end here. Nice work and keep me updated on how I can assist 🥳

Addresses #1167.
In #1166, we found that markdown links across docs instances was going to introduce a ton of friction.
This PR introduces an MDX plugin that should remove this friction. Instead of needing to specify the version of the other docs instance to link to, we can now use a token like
$docs$to have docusaurus interpret the version based on the source file.For example, if we have a link that goes to
$optimize$/what-is-optimize, the target URL will be expanded to....optimize/next/what-is-optimizewhen the source file is in the/docs/folderoptimize/what-is-optimizewhen the source file is in the/versioned_docs/version-8.0folderoptimize/3.7.0/what-is-optimizewhen the source file is in the/versioned_docs/version-1.3folderSee the src/mdx/expandVersionedUrl.spec.js file in this PR for more examples of URL expansion.
My intention is to merge this PR separate from and prior to #1166, to make it easier to review in isolation. (That PR will get very large with all of the content moving around.)
What it looks like in use
Here's a video of the plugin in action.
In addition, #1166 demonstrates the usage:
$docs$or$optimize$to signal that they should be interpreted based on the source location.Implementation notes
$docs$and$optimize$tokens are completely arbitrary. If we want, we can use any other token.