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

Native GitHub linking support #3547

Closed
omry opened this issue Oct 7, 2020 · 35 comments
Closed

Native GitHub linking support #3547

omry opened this issue Oct 7, 2020 · 35 comments
Labels
closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future.

Comments

@omry
Copy link

omry commented Oct 7, 2020

🚀 Feature

It is common to add links to GitHub from project documentation, for example to example applications.
The approach of using a regular link has multiple problems.

  1. GitHub repositories are live and constantly changing, this means that documentation linking master is more likely to break.
    There is no central place to change all the links to a specific branch.
    As an example, when cutting a version of the docs, it is common to have a release branch. It would be great if I could tell the Docusaurus something like "Use 1.0_branch for all the GitHub links in this version" instead of having to fix them one at a time.

  2. Broken links to GitHub are likely the responsibility of the developers, but Docusaurus do not report them during the build process.

What I have in mind is some API to generate GitHub links, that would consult configuration that can be per version.

Motivation

Broken links, broken links everywhere.

Pitch

Developers, developers, developers.

@omry omry added feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. status: needs triage This issue has not been triaged by maintainers labels Oct 7, 2020
@slorber
Copy link
Collaborator

slorber commented Oct 7, 2020

Hi,

  1. GitHub repositories are live and constantly changing, this means that documentation linking master is more likely to break.
    There is no central place to change all the links to a specific branch.
    As an example, when cutting a version of the docs, it is common to have a release branch. It would be great if I could tell the Docusaurus something like "Use 1.0_branch for all the GitHub links in this version" instead of having to fix them one at a time.

We can probably have a markdown preprocessing step (remark plugin) that would do some versioning-aware link replacement. Such tool can already be built in userland (as a docs remark plugin), so if we add this in core this should rather be useful for more than 1 user and ideally decoupled from Github (ie work with other systems like Gitlab too).

What API do you have in mind to express your need?
Can you give some config code samples that you'd like to use?
Can you give concrete examples of pages that should link to specific locations?

  1. Broken links to GitHub are likely the responsibility of the developers, but Docusaurus do not report them during the build process.

It is hard to detect broken links through the build process because unlike internal links, we have to do an actual http request to know if that external link is broken or not.

Also it can give false positives if Github or the network is temporarily unavailable while that build happens.

Note that there are also existing external toolings or SaaS softwares that can enable you to find check broken links. This is not really related to Docusaurus, as detecting a broken link can be done by checking existing urls found in static html documents, so the same tool can be run on top of Gatsby, Docusaurus, Jekyll or whatever...

For example, there's this Netlify build plugin that has an option to check broken external links: https://github.com/Munter/netlify-plugin-checklinks

@omry
Copy link
Author

omry commented Oct 8, 2020

We can probably have a markdown preprocessing step (remark plugin) that would do some versioning-aware link replacement. Such tool can already be built in userland (as a docs remark plugin), so if we add this in core this should rather be useful for more than 1 user and ideally decoupled from Github (ie work with other systems like Gitlab too).

I agree.
I think we can try to ask other people that use versioning if the problem described here resonates with them.

What API do you have in mind to express your need?

SCMLink("README.txt") -> https://github.com/facebookresearch/hydra/blob/master/README.md

If config specifies a branch, it should point to that branch:

SCMLink("README.txt") -> https://github.com/facebookresearch/hydra/blob/1.0_branch/README.md

Which suggests that the pattern is:

https://github.com/{org}/{repo}/blob/{branch}/{path}

Can you give some config code samples that you'd like to use?

The main config already contains the first two and those are stable across versions (although maybe we can also support overriding them to support a repo move between doc versions. this is probably out of scope for an initial version though).

    organizationName: 'facebookresearch',
    projectName: 'hydra',

One way to express this in the config is:

branch: 'master', # default branch, should be overridable per version.
scm_url_pattern: 'https://github.com/{org}/{repo}/blob/{branch}/{path}'

which can later be populated from the config. (obviously the specific templating style is just a suggestion, if you have any existing style for something similar - it should be used here too).

Can you give concrete examples of pages that should link to specific locations?

See above.

It is hard to detect broken links through the build process because unlike internal links, we have to do an actual http request to know if that external link is broken or not.
Also it can give false positives if Github or the network is temporarily unavailable while that build happens.

I think just having an automated way to generate the GitHub links would eliminate a class of such problem where many links are gradually breaking as the master branch is changing.
Build time check is a nice-to-have and could potentially be added later.

Note that there are also existing external toolings or SaaS softwares that can enable you to find check broken links. This is not really related to Docusaurus, as detecting a broken link can be done by checking existing urls found in static html documents, so the same tool can be run on top of Gatsby, Docusaurus, Jekyll or whatever...
For example, there's this Netlify build plugin that has an option to check broken external links: https://github.com/Munter/netlify-plugin-checklinks

Good point, but there is a difference:
SCM links are more likely to break by my own actions, while external links are probably more stable.

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

@omry to me a more generic solution could be to just have a remark plugin that replaces the link target value by another value.

For example.

In md, you could have:

[Readme](README.md)

In config you could have:

module.exports = {
  linkAliases: {
    "README.md": "https://github.com/facebookresearch/hydra/blob/1.0_branch/README.md"
  },   
}

This way you have maximum flexibility to generate all the "link aliases" you want in config, through code. It does not have to be coupled to any SCM system, you just create your aliases the way you want.

@omry
Copy link
Author

omry commented Oct 12, 2020

  1. This would require configuration per link (I have tens, maybe hundreds of such links).
  2. I am not sure how this works across different versions (I want to link to different branch depending on the documentation version).

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

This would require configuration per link (I have tens, maybe hundreds of such links).

At some point, we need to be aware of which links to rewrite and which links to not rewrite so you'll need to maintain an exhaustive list on your side.

You can generate the aliases object using node code.

I am not sure how this works across different versions (I want to link to different branch depending on the documentation version).

We can create one linkAliases config per version as well, so that the same alias is not resolved the same way according to the version.


Can you give concrete examples of pages linking to Github so that I understand better?


What I want to avoid is cluttering the API with options, particularly when it's coupled to a proprietary SCM. So I'd rather give you extension points to implement this feature in userland rather than shipping a high-level API.

This looks to me we could already implement this feature as a remark plugin, and it does not necessarily have to be in core, so I'd rather advocate to build this as a plugin and see if there is enough traction, and if people really find it useful, we'd move it as a default core plugin. This would also help figure out what is a good API in practice, by dogfooding on an existing site, before introducing a badly designed API on which we'd have to do breaking changes.

@omry
Copy link
Author

omry commented Oct 12, 2020

For starters, I understand that tension between adding new APIs and reusing existing ones.
Implementing this as a kind of plugin sounds like a good idea, but at the same time it could maybe be generalized as a "Templated link" which can again be in a core or as a plugin.

At some point, we need to be aware of which links to rewrite and which links to not rewrite so you'll need to maintain an exhaustive list on your side.

This can be achieved in many ways, an exhaustive list in the config does not sound like a good one to me.
The involved work is not much different than me going and editing all the links manually for each version.
There could also be another case where I want README.md to not be rewritten somewhere else in the website.

Alternative possible approached here:

  • Using an explicit function call (that can be configured per version)
  • Using a custom React tag (that can be configured per version)

I do not have enough background in React/JS development to know if any of these approaches can be implemented as a plugin.

@slorber
Copy link
Collaborator

slorber commented Oct 14, 2020

Using an explicit function call (that can be configured per version)

In the function you still need to tell if you need to rewrite or not the link, so you've be able, for each link, to return the same value, or return the replaced link. How do you do this? Can you give an example fn implementation that you'd use on your site? Do links to rewrite have a proper way to identify themselves? You could maybe use a prefix like [MyDocPage](githubBranch:///docs/myDoc) or something.


Using a custom React tag (that can be configured per version)

Instead of providing an opinionated component, we could enable you to build your own.

import Link from "@docusaurus/Link"

expert default function MyGithubVersionedLink(props) {
  const version = useVersion();
  const gitBranch = version ? `branch-${version.name}` : "master";
  const versionedTo = `https://github.com/facebookresearch/hydra/blob/${gitBranch}${props.to}`
  return <Link {...props} to={versionedTo}/>
}

This is generic, and let you do whatever linking strategy you want. The only missing piece is the useVersion() hook that currently does not exist.


Shipping a lot of opinionated things gives us the burden to maintain these things over time. I think we really want to avoid this (except for core features), and keep a logic of composition, enabling you to build your own opinionated things on top of Docusaurus.

I think we should only add opinionated things if there is enough traction for it, and if it's proven to be a burden or impossible to build in userland.

@omry
Copy link
Author

omry commented Oct 14, 2020

In the function you still need to tell if you need to rewrite or not the link, so you've be able, for each link, to return the same value, or return the replaced link. How do you do this? Can you give an example fn implementation that you'd use on your site? Do links to rewrite have a proper way to identify themselves? You could maybe use a prefix like MyDocPage or something.

If this is an explicit function call it can just always rewrite the url (it should not be called otherwise).


This is interesting, questions inline.

import Link from "@docusaurus/Link"
// export?
expert default function MyGithubVersionedLink(props) {
  const version = useVersion();
  const gitBranch = version ? `branch-${version.name}` : "master";
  // can I access the global config to get facebookresearch and hydra without hard coding them?
  const versionedTo = `https://github.com/facebookresearch/hydra/blob/${gitBranch}${props.to}`
  return <Link {...props} to={versionedTo}/>
}

The most important question: how would that be used on the callsite?
Following the example from Link:

<Link to="/blog">blog</Link>

Will that become:

<MyGithubVersionedLink to="/blog">blog</MyGithubVersionedLink>

?

@slorber
Copy link
Collaborator

slorber commented Oct 15, 2020

If this is an explicit function call it can just always rewrite the url (it should not be called otherwise).

You probably don't want to rewrite this URL if it's a link to another document, or the homepage. This function can potentially break your site internal links if you don't use it carefully., as you have the ability to rewrite / to /doesNotExist or whatever.


Yes exactly, this should work.

import MyGithubVersionedLink from "@site/src/components/MyGithubVersionedLink"

<MyGithubVersionedLink to="/blog">blog</MyGithubVersionedLink>

We could eventually find a way for you to provide the component globally to all MDX files, so that you don't have to import it everywhere, but the above should already work as of today (apart the useVersion that does not exist).

@omry
Copy link
Author

omry commented Oct 15, 2020

Okay, this sounds good to me.
I think we are just missing the ability to set user metadata per version, that preferably overrides user metadata sets in the config and access it, something like:

versions config:

default:
  branch: master
1.0:
  branch: 1.0_branch
1.1:
   foo: bar # does not set branch
vm = version_metadata()
branch = vm.get_value("branch") 
# branch is:
#   master on "next"
#   1.0_branch on 1.0
#   master on 1.1 (not specified so using default instead).

get_value(key) can look like:

if key in metadata[current_version]:
   return metadata[current_version][key]
else:
   return metadata["default"][key]

If you do not think get_value should be a part of the core I just need access to the metadata dictionary and to the current version string.

@slorber
Copy link
Collaborator

slorber commented Oct 16, 2020

You can pass additional global data to your site by using the "customFields" of the config:
https://v2.docusaurus.io/docs/docusaurus.config.js/#customfields

So basically, you can pass a map versionName->customVersionConfig.

And access it globally to do whatever you want in your custom Link or other components:

import Link from "@docusaurus/Link"

expert default function MyGithubVersionedLink(props) {
  // read current version (will only work on versioned pages, not homepage for exemple)
  const version = useVersion();

  // Access your custom versionName -> cfg mapping
  const {customVersionConfigs} = useDocusaurusContext().siteConfig.customFields;

  // Access the custom config of current version
  const customVersionConfig = customVersionConfigs[version.name];

  return <Link {...props} to={`${customVersionConfig.gitBranchPrefix}${props.to}`}/>
}

Eventually, we could allow to assign custom data per version, so that it's more easily readable.

import Link from "@docusaurus/Link"

expert default function MyGithubVersionedLink(props) {
  const version = useVersion();
  const customVersionConfig = version.customData.myConfig;
  return <Link {...props} to={`${customVersionConfig.gitBranchPrefix}${props.to}`}/>
}

Do we agree that you only plan to put such link on the versioned docs right? Because if you use it on the homepage, we don't really know which version you are browsing as the homepage is not versioned (in such case you have to provide a default version to link to)

@omry
Copy link
Author

omry commented Oct 17, 2020

I think if we don't know the version as can default to master somehow.
otherwise this looks good.

By the way, what do you want to call the function useVersion() and not something like getVersion() or getVersionMetadata()?

@slorber
Copy link
Collaborator

slorber commented Oct 17, 2020

Because we read the version from context through a hook, so the function would also be a React hook, that all start with use by convention

@omry
Copy link
Author

omry commented Dec 29, 2020

Any plans of addressing the functional gaps we identified as needed to support this?

@slorber
Copy link
Collaborator

slorber commented Dec 30, 2020

I've added some infra that could be useful recently with #3949
It may not be exactly what you want in its current form.

But if you were able to config an edit URL per version, does it work for your usecase? (you could use different branches for each version)

@omry
Copy link
Author

omry commented Dec 30, 2020

I know it's been a while since there was activity here so let me refresh your memory:

The problem is that I want GitHub links in different versions of the docs to point to the corresponding branch without having to edit each and every one of the links. (this is something I can already do by editing the versioned snapshots of the documents).
In a quick check, I currently have 90 such links in a single version, so that's a lot of manual work.

We have discussed ways to support it, and I think we concluded that allowing a per version metadata would enable me to create code that would generate the GitHub link based on the metadata in the current version.

@slorber
Copy link
Collaborator

slorber commented Dec 31, 2020

Yes, the per-version metadata is what I'm talking about.

Can you give me a more concrete example?
Like, give me some doc links from your site, and tell me where the edit buttons should link to?

@omry
Copy link
Author

omry commented Dec 31, 2020

I am confused by you asking for a concrete example with the edit buttons.
this feature request is about the GitHub links.

In https://hydra.cc/docs/next/tutorials/basic/your_first_app/simple_cli (version: Next)
There is a link to https://github.com/facebookresearch/hydra/tree/master/examples/tutorials/basic

That link should be:
Version 1.0: https://github.com/facebookresearch/hydra/tree/1.0_branch/examples/tutorials/basic

Once I create the docs for 1.1 from master, I will want to specify 1.1_branch in the metadata for that docs version, and I want the above link to become https://github.com/facebookresearch/hydra/tree/1.1_branch/examples/tutorials/basic

@slorber
Copy link
Collaborator

slorber commented Jan 11, 2021

Sorry, now I remind better the problem 👍

Actually, there is a hook that you could use today already to build your custom link

import {useActiveVersion} from '@theme/hooks/useDocs';

export default function MyGithubVersionedLink(props) {
  const activeVersion = useActiveVersion();
  return <Link {...props} to={createMyCustomUrl(props,activeVersion)}/>
}

We do not provide a way to assign custom data to a given version, but you can do that using the customFields in config https://v2.docusaurus.io/docs/next/docusaurus.config.js/#customfields

Can you tell me how far you can go with the current apis?

@omry
Copy link
Author

omry commented Jan 11, 2021

Is there a way for me to determine the current version?
In principle I should be able to create a map for a custom field.

module.exports = {
  customFields:  {
    # maps version string to a corresponding GitHub branch
    github_branch: {
      1.0: "1.0_branch",
      next: "master"
    },
  },
};

@slorber
Copy link
Collaborator

slorber commented Jan 11, 2021

The current version is useActiveVersion(), it will return a different result depending on the route/url you are on (including undefined, so beware!)

@omry
Copy link
Author

omry commented Jan 11, 2021

Great, will try that. (could be a while).

@omry
Copy link
Author

omry commented Jan 16, 2021

just spent an hour or two trying to make this work. My JavaScript and React foo are not sufficient. I didn't get too far.

Can you help with some basic things?
Where does the proposed hook snippet goes?

import {useActiveVersion} from '@theme/hooks/useDocs';

export default function MyGithubVersionedLink(props) {
  const activeVersion = useActiveVersion();
  return <Link {...props} to={createMyCustomUrl(props,activeVersion)}/>
}

I tried adding it directly in the md file, and also in a component in src/components/GitHubLink.jsx.
couldn't get any of those options to really work.

Sorry for the noobish question.

@slorber
Copy link
Collaborator

slorber commented Jan 18, 2021

Hi @omry

Here you will find a Codesandbox example with what you want to do:
https://codesandbox.io/s/competent-galileo-8lf20?file=/src/components/GithubLink.js

Let me know if this is good enough for your usecase

@omry
Copy link
Author

omry commented Jan 18, 2021

@slorber , this looks awesome. Thanks so much for taking the time to create that example!

@omry
Copy link
Author

omry commented Jan 18, 2021

I think it's very close.

Here are a few issues I am seeing:

  1. In yarn build I see Component GithubLink was not imported, exported, or provided by MDXProvider as global scope.
    Not sure if this is significant or not.

  2. I have the following construct:

[![Example](https://img.shields.io/badge/-Example-informational)](https://github.com/facebookresearch/hydra/blob/master/examples/tutorials/basic/your_first_hydra_app/1_simple_cli/my_app.py)

You can see it at the top of my pages, the blue "example" box.
I was not able to use the GithubLink with it. I also tried to somehow call the javascript function directly without much luck.

Here is my PR, I have made some small changes to make the function creating the url easier to use directly (but was not able to call it directly).

@slorber
Copy link
Collaborator

slorber commented Jan 19, 2021

Hi @omry

Your link is not just a regular link but a link + an image.

You can interleave markdown in JSX but it needs to add line breaks around the markdown (may be fixed in MDX 2.0 upgrade)

The 1st error is likely due to your import not being taken into account (also worth adding line breaks around the import).

You'll find an updated Sandbox here with your example badge: https://codesandbox.io/s/hardcore-edison-e7i7i

Please fork this initial sandbox and see what blocks you from there and send me back an updated sandbox.
Working with Hydra local branches takes more time for me to load everything locally compared to a codesandbox

@omry
Copy link
Author

omry commented Jan 20, 2021

@slorber, this is great.
I am porting my links to those two forms (components) and so far I didn't run into any issues.

Thanks again for your help.
Feel free to close, although I think it's worth considering making the example more visible somehow as this is pretty common scenario.

@slorber
Copy link
Collaborator

slorber commented Jan 20, 2021

What do you mean by "making the example more visible"?

If you feel it should be documented somewhere, do you have a concrete suggestion in mind?

I think we need some kind of recipes category to show how to solve specific problems, and that could be in. Just afraid this section would be quite messy and not sure how it should be organized.

@omry
Copy link
Author

omry commented Jan 20, 2021

  1. I ran into some layout issues with the custom component. here is what it looks like.
    Any advice is appreciated :).

  2. What do you mean by "making the example more visible"?
    If you feel it should be documented somewhere, do you have a concrete suggestion in mind?
    I think we need some kind of recipes category to show how to solve specific problems, and that could be in. Just afraid this section would be quite messy and not sure how it should be organized.

For Hydra, I have a dedicated section called Common Patterns I use to show explain and demonstrate higher level concepts.
Here is an example page.
The pages there contains a link to a working example.

You could do something similar, and maybe use codesandbox.io link as the working example link.
not sure how this approach would handle docusaurus upgrades and how to automatically test that the example works.

@slorber
Copy link
Collaborator

slorber commented Jan 21, 2021

  1. unfortunately interleaving md and jsx has its own set of shortcomings, as interleaved markdown requires spaces around the md and add extra <p> tags around it so there are sometimes unwanted line breaks

For better control it's better go full JSX when needed:

<>
All the inline links: <a href="https://img.shields.io/pypi/v/hydra-colorlog"><img src="https://img.shields.io/pypi/v/hydra-colorlog"/></a>
<GithubLinkExample to="/examples/tutorials/basic/your_first_hydra_app/1_simple_cli/my_app.py"/>
<GithubLinkExample to="/examples/tutorials/basic/your_first_hydra_app/1_simple_cli/my_app.py"/>
</>

image

If certain patterns are common I'd recommend using a custom component to avoid duplication

  1. will think about it, thanks

@omry
Copy link
Author

omry commented Jan 21, 2021

Woa, had no idea about <> and </> to switch into JSX mode.
Any hints about matching the spacing of the image tag in normal MD?

@slorber
Copy link
Collaborator

slorber commented Jan 22, 2021

@omry this is just a matter of reproducing exactly the same HTML output, with MD or JSX.
Markdown seems to add whitespace in the HTML output, that you can replicate in JSX.

This is the desited layout:  
[![PyPI](https://img.shields.io/pypi/v/hydra-colorlog)](https://pypi.org/project/hydra-colorlog/)
![Example](https://img.shields.io/badge/-Example-informational)
![Example](https://img.shields.io/badge/-Example-informational)

<p>
This is the desited layout:<br/>
  <a href="https://img.shields.io/pypi/v/hydra-colorlog">
    <img src="https://img.shields.io/pypi/v/hydra-colorlog"/>
  </a>
  {" "}
  <img src="https://img.shields.io/badge/-Example-informational"/>
  {" "}
  <img src="https://img.shields.io/badge/-Example-informational"/>
</p>

image

The MD syntax is just a "shortcut" to JSX, at the end MDX is compiled to JSX.

You can use the MDX playground to see how it works: https://mdxjs.com/playground/

image

image

@omry
Copy link
Author

omry commented Jan 22, 2021

the MDX playgound is great.
Thanks for all your help with my noobish questions!

@slorber
Copy link
Collaborator

slorber commented Jan 27, 2021

great, I'll close this issue then ;)

@slorber slorber closed this as completed Jan 27, 2021
@Josh-Cena Josh-Cena added closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) and removed status: needs triage This issue has not been triaged by maintainers labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future.
Projects
None yet
Development

No branches or pull requests

3 participants