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

feat(v2): add ability to set custom heading id #4222

Merged
merged 15 commits into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/docusaurus-mdx-loader/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ const mdx = require('@mdx-js/mdx');
const emoji = require('remark-emoji');
const matter = require('gray-matter');
const stringifyObject = require('stringify-object');
const slug = require('./remark/slug');
const headings = require('./remark/headings');
const toc = require('./remark/toc');
const transformImage = require('./remark/transformImage');
const transformLinks = require('./remark/transformLinks');

const DEFAULT_OPTIONS = {
rehypePlugins: [],
remarkPlugins: [emoji, slug, toc],
remarkPlugins: [emoji, headings, toc],
};

module.exports = async function docusaurusMdxLoader(fileString) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
* LICENSE file in the root directory of this source tree.
*/

/* Based on remark-slug (https://github.com/remarkjs/remark-slug) */
/* Based on remark-slug (https://github.com/remarkjs/remark-slug) and gatsby-remark-autolink-headers (https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-remark-autolink-headers) */

/* eslint-disable no-param-reassign */

import remark from 'remark';
import u from 'unist-builder';
import removePosition from 'unist-util-remove-position';
import toString from 'mdast-util-to-string';
import visit from 'unist-util-visit';
import slug from '../index';

function process(doc, plugins = []) {
Expand All @@ -27,7 +29,7 @@ function heading(label, id) {
);
}

describe('slug plugin', () => {
describe('headings plugin', () => {
test('should patch `id`s and `data.hProperties.id', () => {
const result = process('# Normal\n\n## Table of Contents\n\n# Baz\n');
const expected = u('root', [
Expand Down Expand Up @@ -157,7 +159,7 @@ describe('slug plugin', () => {
expect(result).toEqual(expected);
});

test('should create GitHub slugs', () => {
test('should create GitHub-style headings ids', () => {
const result = process(
[
'## I ♥ unicode',
Expand Down Expand Up @@ -225,7 +227,7 @@ describe('slug plugin', () => {
expect(result).toEqual(expected);
});

test('should generate slug from only text contents of headings if they contains HTML tags', () => {
test('should generate id from only text contents of headings if they contains HTML tags', () => {
const result = process('# <span class="normal-header">Normal</span>\n');
const expected = u('root', [
u(
Expand All @@ -244,4 +246,58 @@ describe('slug plugin', () => {

expect(result).toEqual(expected);
});

test('should create custom headings ids', () => {
const result = process(`
# Heading One {#custom_h1}

## Heading Two {#custom-heading-two}

# With *Bold* {#custom-withbold}

# Snake-cased ID {#this_is_custom_id}

# No custom ID

# {#id-only}

# {#text-after} custom ID
`);

const headers = [];
visit(result, 'heading', (node) => {
headers.push({text: toString(node), id: node.data.id});
});

expect(headers).toEqual([
{
id: 'custom_h1',
text: 'Heading One',
},
{
id: 'custom-heading-two',
text: 'Heading Two',
},
{
id: 'custom-withbold',
text: 'With Bold',
},
{
id: 'this_is_custom_id',
text: 'Snake-cased ID',
},
{
id: 'no-custom-id',
text: 'No custom ID',
},
{
id: 'id-only',
text: '{#id-only}',
},
{
id: 'text-after-custom-id',
text: '{#text-after} custom ID',
},
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
* LICENSE file in the root directory of this source tree.
*/

/* Based on remark-slug (https://github.com/remarkjs/remark-slug) */
/* Based on remark-slug (https://github.com/remarkjs/remark-slug) and gatsby-remark-autolink-headers (https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-remark-autolink-headers) */

const visit = require('unist-util-visit');
const toString = require('mdast-util-to-string');
const slugs = require('github-slugger')();

const customHeadingIdRegex = /^(.*?)\s*\{#([\w-]+)\}$/;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about extracting customHeadingIdRegex.exec(heading); in a separate function const {heading,id} = extractHeadingId(rawHeading) and adding tests?

That would simplify the testing of Regepx edge cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but we have separate test that with different cases, so it seems to me redundant extract new function (and its test), because in many ways it will duplicate this test.


function slug() {
const transformer = (ast) => {
slugs.reset();
Expand All @@ -26,11 +28,29 @@ function slug() {
const headingTextNodes = headingNode.children.filter(
({type}) => !['html', 'jsx'].includes(type),
);
const normalizedHeadingNode =
const heading = toString(
headingTextNodes.length > 0
? {children: headingTextNodes}
: headingNode;
id = slugs.slug(toString(normalizedHeadingNode));
: headingNode,
);

// Support explicit heading IDs
const customHeadingIdMatches = customHeadingIdRegex.exec(heading);

if (customHeadingIdMatches) {
Copy link
Contributor Author

@lex111 lex111 Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if this feature will have to be configurable? How much of an impact on performance would it have if it were always enabled by default? Or does it make sense to create a separate plugin for this feature? For now I have combined it into the existing slug plugin because they essentially do the same thing (create headings id).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to enable by default and put in mdx loader.
We likely want to encourage users to have stable anchor ids and this should work consistently for docs, blog and pages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought that having new plugin might be better for this. Since this feature is mainly needed when using the i18n. Moreover, I think to add a cli command to automatically create explicit ids for existing headers (we have extendCli hook in plugin lifecycle). Because currently I do not understand how I can add a new cli command inside the mdx-loader package. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a way to "lock" the ids at a given time looks like a nice idea.

Not sure how to build that easily, how would the plugin know where to look for markdown files, as the paths are provided as options to the docs, blog, page plugin?

The only thing I'm thinking of is having this cli in core, and getting all the relevant paths from getPathsToWatch (as you basically watch the md files, it will work but feel a bit hacky at the same time)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this feature is mainly needed when using the i18n.

I think it's actually useful in all cases. If you want to link to some heading, we'd rather ensure that link does not break when the heading is updated.

Fixing heading ids you want to link to should rather be the norm, and we could even add a warning someday to tell the user that he's linking to an anchor that has not been fixed?

Currently it's too easy to have broken anchor links in a Docusaurus site.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lost sight of that it is currently not possible to add new Remark plugin from the custom Docusaurus plugin. Maybe we should create new hook for this purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to add new command to write heading ids to the docusaurus core (eg. yarn docusaurus write-heading-ids community). Its implementation is very simple, so it might be worth improving it.

id = customHeadingIdMatches[2];

// Remove the custom ID part from the text node
if (headingNode.children.length > 1) {
headingNode.children.pop();
} else {
const lastNode =
headingNode.children[headingNode.children.length - 1];
lastNode.value = customHeadingIdMatches[1] || heading;
}
} else {
id = slugs.slug(heading);
}
}

data.id = id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import remark from 'remark';
import mdx from 'remark-mdx';
import vfile from 'to-vfile';
import plugin from '../index';
import slug from '../../slug/index';
import headings from '../../headings/index';

const processFixture = async (name, options) => {
const path = join(__dirname, 'fixtures', `${name}.md`);
const file = await vfile.read(path);
const result = await remark()
.use(slug)
.use(headings)
.use(mdx)
.use(plugin, options)
.process(file);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import remark from 'remark';
import mdx from 'remark-mdx';
import vfile from 'to-vfile';
import plugin from '../index';
import slug from '../../slug/index';
import headings from '../../headings/index';

const processFixture = async (name, options) => {
const path = join(__dirname, 'fixtures', `${name}.md`);
const file = await vfile.read(path);
const result = await remark()
.use(slug)
.use(headings)
.use(mdx)
.use(plugin, {...options, filePath: path})
.process(file);
Expand Down
4 changes: 4 additions & 0 deletions website/docs/guides/docs/docs-create-doc.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ The headers are well-spaced so that the hierarchy is clear.
- that you want your users to remember
- and you may nest them
- multiple times

### Custom id headers {#custom-id}

With `{#custom-id}` syntax you can set your own header id.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth creating a page dedicated to anchor links in markdown features category?

I'd also document this in the i18n section but I can do it in another PR?

```

This will render in the browser as follows:
Expand Down
2 changes: 2 additions & 0 deletions website/src/pages/examples/markdownPageExample.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,5 @@ import MyComponentSource from '!!raw-loader!@site/src/pages/examples/\_myCompone
<CodeBlock className="language-jsx">{MyComponentSource}</CodeBlock>

</BrowserWindow>

## Custom heading id {#custom}