keystonejs / keystone

The superpowered headless CMS for Node.js — built with GraphQL and React
https://keystonejs.com
MIT License
9.18k stars 1.15k forks source link

Internal links in docs either work on the docs site OR GitHub (but not both) #2041

Closed molomby closed 4 years ago

molomby commented 4 years ago

Right now a lot (all?!) links in the project docs that reference other internal docs or packages will either work on the docs site OR work locally and on GitHub. Eg..

Doc site URL Local path and GitHub URL
/guides/authentication /docs/guides/authentication.md
/keystonejs/auth-password /packages/auth-password/
/api/access-control /docs/api/access-control.md
../../guides/access-control ../../docs/guides/access-control.md

This is due to a divergence between the physical path of the Markdown files and the published URLs. The problem effects relative and absolute paths. File extension and some edge cases around deep linking come into it too. (The local path and GitHub URL also diverge slightly due to GitHub defaulting to README.md as an "index" file.)

I see this as a pretty serious problem. Obviously we want a nice docs site to keep Keystone accessible but surely having these links make sense when you're in the code or on GitHub is critical.

It's has been picked up before (#902) and patched in favour of the docs site (but maybe the full implications weren't realised at that point?). More recently, I went though "fixing" a stack of links before realising the implications 😔

Solutioning

We need to pick a canonical path schema for our links and I think it has to be based on the physical file paths. The docs site is generated, which gives us the chance to map physical paths to "doc site URLs". Having the file paths in the raw Markdown is the only way to solve for Keystone developers who are seeing the links either locally in their code editor or on GitHub. I also suspect that without file paths in Markdown files, automated testing of Markdown links (#892) will be much more difficult.

Beyond that, there's clearly a mapping between the file paths and the doc URLs. Whether this is extracted from the existing [meta] headers or stored elsewhere, it should be applied to links during the publishing process.

timleslie commented 4 years ago

We need to pick a canonical path schema for our links and I think it has to be based on the physical file paths

Strongly agree! ☝️

molomby commented 4 years ago

Reframing this slightly... our Markdown files are currently consumed in several way:

They need to be written in a shared format. This issue is about deciding on the link schema for that shared format. Whatever we come up with will need to support the targets listed above.

Our doc targets have different requirements and are flexible in different ways. We've already identified some of differences between the docs site and GitHub above. But there are also important difference between the other targets:

The question here then becomes..

What schema/system/format can we devise for our links (and Markdown docs generally) such that they work well on all doc targets.

I'm leaning towards saying that..

Note these rules would still requires work to be done to remap paths for the doc site (but that's completely possible). Also, in case it wasn't obvious, I'm reeeally hoping to get automated tests enforcing these (or other workable) rules.

molomby commented 4 years ago

@Noviny and @mitchellhamilton! Sounded like you two had ideas..?

Noviny commented 4 years ago

We have a bunch of implementation options within these design constraints. Next step is probably just clearing some time to implement it.

molomby commented 4 years ago

This was discussed internally and the decision was made to adopt the following guidelines:

General Guidelines

Docs Site

NPM Packages

There was talk of swapping package readmes out for a stub (with just the Keystone header) as part of the publishing process. This would address any issues where packages docs link outside their dir in the codebase (these links will break once published). As yet, this doesn't occur very often though. We'll likely revisit doc conversion around package publishing later on.

molomby commented 4 years ago

Further to the issue with NPM packages mentioned above...

This would address any issues where packages docs link outside their dir in the codebase (these links will break once published). As yet, this doesn't occur very often though. We'll likely revisit doc conversion around package publishing later on.

This can be seen on (for example) the @keystonejs/access-control package.

The docs use the correct "relative to the repo root" URL formulation:

.. please consult the [Access Control Guide](/docs/guides/access-control.md).

But when published to NPM this breaks, resulting in a url of...

https://github.com/keystonejs/keystone/tree/master/packages/access-control/blob/HEAD/docs/guides/access-control.md`

This is due to the packages repository config:

  "repository": "https://github.com/keystonejs/keystone/tree/master/packages/access-control"

If we adopt the object syntax here and specify a directory, this problem goes away ✨, eg..

  "repository": {
    "type" : "git",
    "url" : "https://github.com/keystonejs/keystone.git",
    "directory": "packages/access-control"
  }

This is indeed the recommended format for monorepos:

If the package.json for your package is not in the root directory (for example if it is part of a monorepo), you can specify the directory in which it lives.

I'm creating a PR for this now, just documenting for posterity.