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

RFC: plugin Markdown lifecycles – configureMarkdownLoader #6370

Open
2 tasks done
Josh-Cena opened this issue Jan 15, 2022 · 2 comments
Open
2 tasks done

RFC: plugin Markdown lifecycles – configureMarkdownLoader #6370

Josh-Cena opened this issue Jan 15, 2022 · 2 comments
Labels
domain: markdown Related to Markdown parsing or syntax proposal This issue is a proposal, usually non-trivial change
Milestone

Comments

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jan 15, 2022

Have you read the Contributing Guidelines on issues?

Motivation

A more solid proposal derived from #4625. I propose two Markdown-related lifecycles for plugins: configureMarkdownLoader and getMarkdownComponents.

configureMarkdownLoader

Status quo

Currently, every content plugin uses @docusaurus/mdx-loader and passes it through configureWebpack. The docs and blog plugins also host their own Markdown loaders, in order to access the file path -> URL mapping and convert MD links to URL links.

However, this means each plugin's data is completely sandboxed, all the way from content loading to route creation. A plugin can't access routes created by another plugin at any stage in its lifespan. In addition, this means duplicated logic in every plugin. After #5999, the remark plugins will be moved to a global config option, which has to be hooked into in every plugin.

This also means that linkification happens before any Remark plugin gets loaded, and people can't remap Markdown paths, as requested in #4039, because when the beforeDefaultRemarkPlugins get loaded, the Markdown links don't exist any more.

Resolution

I propose that we move the MDX loader to the core, just like the CSS loader or JS loader. This effectively means we treat MD as a first-class citizen in our architecture. And then, each plugin will pass in loader options that are aggregated together. This solves several problems:

  • Plugins can access routes created by the others through an aggregated global filePathToPermalink map.
  • The admonitions plugin doesn't have to be strangely injected from preset options into the plugin, but the theme can directly register remark plugins that are accessible for all content plugins. (The admonitions plugin being strongly coupled to the classic theme is actually very bad design, but in the future, we may create other theme-specific remark plugins?)
  • We can easily create more global Markdown configurations in the future, even swapping out Markdown parsers or inserting additional middlewares, without duplicating logic across content plugins.

For example:

function pluginContentDocs() {
  return {
    configureMarkdown(content) {
      return {
        isIncluded: (filePath) => {
          return content.allDocSources.includes(filePath);
        },
        metadataPath: (filePath) => {
          const aliasedPath = aliasedSitePath(mdxPath, siteDir);
          return path.join(dataDir, `${docuHash(aliasedPath)}.json`);
        },
        // Allows global linkification
        getPermalink: (filePath) => {
          return allDocs.find(doc => doc.source === filePath).permalink;
        },
        isMDXPartial: (filePath) => {
          return createAbsoluteFilePathMatcher(options.exclude, contentDirs)(filePath);
        },
        // Stuff like remarkPlugins, staticDirs, etc. are no longer needed in plugin config
      };
    },
  };
}

The only problem is that some logic is ultimately plugin-specific. For example, how do we handle file-specific logic if the file queried is not handled by this plugin? My current solution is an isIncluded function. All configs are aggregated in a list, and for each file, the isIncluded callback is first called. If it returns true, then the rest of the callbacks are executed. All callbacks are called with filePath, but maybe we can also pass in fileContent if there's a use?

`getMarkdownComponents` is now unnecessary

getMarkdownComponents

Status quo

Currently, the classic theme registers global Markdown components through the MDXComponents. This is problematic, because other themes can't register components on top of this unless we solve #4530. Moreover, the API surface is very implicit to the user. This is ultimately just a register, not a component, and can't be wrapped like other components. (Although you can still use object spreading)

Resolution

I propose a new getMarkdownComponents that's like getThemePath. It returns a path to a component map, in the same shape as the current MDXComponents. The core would merge all component registers and generate it as a temp file. Anything that uses the MDXProvider just needs to import from the generated folder.

After some more thoughts, I don't think we need the getMarkdownComponents API. Instead, we will solve #4530, and allow each theme to wrap the previous one's MDXComponents (which I'm also considering to be refactored into a useMDXGlobals hook).

Self-service

  • I'd be willing to do some initial work on this proposal myself.
@Josh-Cena Josh-Cena added the proposal This issue is a proposal, usually non-trivial change label Jan 15, 2022
@Josh-Cena Josh-Cena changed the title RFC: plugin Markdown lifecycles – configureMarkdownLoader, getMarkdownComponents RFC: plugin Markdown lifecycles – configureMarkdownLoader Feb 13, 2022
@Josh-Cena
Copy link
Collaborator Author

The shiki-twoslash preset uses a mutating siteConfig hack to inject the twoslash remark plugin into the content plugins. This lifecycle can make that a non-hack.

https://github.com/shikijs/twoslash/blob/4c6416670553b61074cd33be0f3bb0fed64d0ada/packages/docusaurus-preset-shiki-twoslash/index.js#L44-L52

@Josh-Cena
Copy link
Collaborator Author

I've closed #6261 because parsing titles and descriptions seems to sacrifice too much performance. Interlinking pages with Markdown paths is still a problem and we have several tests that currently have incorrect outputs. With this proposal, we will be able to move this logic to the MDX loader and thus be able to work on an already parsed MDAST instead of raw strings.

@Josh-Cena Josh-Cena modified the milestones: 2.x, 2.0.0 Mar 16, 2022
@Josh-Cena Josh-Cena added the domain: markdown Related to Markdown parsing or syntax label Mar 31, 2022
@Josh-Cena Josh-Cena modified the milestones: 2.0.0, 3.0.0 Apr 6, 2022
@slorber slorber modified the milestones: Planned, 3.0 Aug 17, 2023
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
Fix all markdown links and configure the Docusaurus build to fail on
broken links.

Compared to before there are two main differences for links within
Docusuaurus:
* Links between URL links need to be URL links to be resolved. This will
  be fixed in Docusaurus 3 [1].
* Links that point to files outside the "docusaurus" directory need to
  be absolute links to be resolvable in the final website. This could
  mabye be improved by adding support for variables [2].

[1]: facebook/docusaurus#6370
[2]: facebook/docusaurus#395

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
Fix all Markdown links and configure the Docusaurus build to fail on
broken links.

Compared to before there are two main differences for links within
Docusuaurus:
* Links between URL links need to be URL links to be resolved. This will
  be fixed in Docusaurus 3 [1].
* Links that point to files outside the "docusaurus" directory need to
  be absolute links to be resolvable in the final website. This could
  mabye be improved by adding support for variables [2].

[1]: facebook/docusaurus#6370
[2]: facebook/docusaurus#395

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
Fix all Markdown links and configure the Docusaurus build to fail on
broken links.

Compared to before there are two main differences for links within
Docusuaurus:
* Links between URL links need to be URL links to be resolved. This will
  be fixed in Docusaurus 3 [1].
* Links that point to files outside the "docusaurus" directory need to
  be absolute links to be resolvable in the final website. This could
  mabye be improved by adding support for variables [2].

[1]: facebook/docusaurus#6370
[2]: facebook/docusaurus#395

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
In Docusaurus 2 links between different plugin instances need to be URL
links which can only be resolved within the built website, but not in
the raw files. Note that this is supposed to be fixed in Docusaurus 3
[1].

[1]: facebook/docusaurus#6370

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
Fix all Markdown links and configure the Docusaurus build to fail on
broken links.

Compared to before there are two main differences for links within
Docusuaurus:
* Links between URL links need to be URL links to be resolved. This will
  be fixed in Docusaurus 3 [1].
* Links that point to files outside the "docusaurus" directory need to
  be absolute links to be resolvable in the final website. This could
  mabye be improved by adding support for variables [2].

[1]: facebook/docusaurus#6370
[2]: facebook/docusaurus#395

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
In Docusaurus 2 links between different plugin instances need to be URL
links which can only be resolved within the built website, but not in
the raw files. Note that this is supposed to be fixed in Docusaurus 3
[1].

[1]: facebook/docusaurus#6370

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
Fix all Markdown links and configure the Docusaurus build to fail on
broken links.

Compared to before there are two main differences for links within
Docusuaurus:
* Links between URL links need to be URL links to be resolved. This will
  be fixed in Docusaurus 3 [1].
* Links that point to files outside the "docusaurus" directory need to
  be absolute links to be resolvable in the final website. This could
  mabye be improved by adding support for variables [2].

[1]: facebook/docusaurus#6370
[2]: facebook/docusaurus#395

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this issue Sep 15, 2023
In Docusaurus 2 links between different plugin instances need to be URL
links which can only be resolved within the built website, but not in
the raw files. Note that this is supposed to be fixed in Docusaurus 3
[1].

[1]: facebook/docusaurus#6370

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
@slorber slorber removed this from the 3.0 milestone Oct 8, 2023
@slorber slorber added this to the 3.x milestone Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: markdown Related to Markdown parsing or syntax proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

No branches or pull requests

2 participants