fix(v2): fix title logic (meta vs heading) + ignore fixed anchor id syntax (#4688)

* parseMarkdownContentTitle should ignore {#my-anchor-id} syntax

* use frontMatter.title in priority for page meta title

* parseMarkdownString should ignore fixed anchor ids syntax

* docs: make the distinction between headingTitle + metaTitle more clear + add useful todo

* docs: make the distinction between headingTitle + metaTitle more clear + add useful todo

* writeHeadingIds should ignore top-level md title like "# Title"
=> we are not supposed to create anchor links for h1 headers

* update tests

* fix doc tests

packages/docusaurus-plugin-content-docs/src/__tests__/__snapshots__/index.test.ts.snap

@@ -146,7 +146,7 @@ Object {
   \\"unversionedId\\": \\"foo/bar\\",
   \\"id\\": \\"foo/bar\\",
   \\"isDocsHomePage\\": false,
-  \\"title\\": \\"Bar\\",
+  \\"title\\": \\"Remarkable\\",
   \\"description\\": \\"This is custom description\\",
   \\"source\\": \\"@site/docs/foo/bar.md\\",
   \\"sourceDirName\\": \\"foo\\",
@@ -182,7 +182,7 @@ Object {
   },
   \\"sidebar\\": \\"docs\\",
   \\"previous\\": {
-    \\"title\\": \\"Bar\\",
+    \\"title\\": \\"Remarkable\\",
     \\"permalink\\": \\"/docs/foo/bar\\"
   },
   \\"next\\": {
@@ -396,7 +396,7 @@ Object {
             \\"items\\": [
               {
                 \\"type\\": \\"link\\",
-                \\"label\\": \\"Bar\\",
+                \\"label\\": \\"Remarkable\\",
                 \\"href\\": \\"/docs/foo/bar\\"
               },
               {

packages/docusaurus-plugin-content-docs/src/__tests__/docs.test.ts

@@ -181,7 +181,7 @@ describe('simple site', () => {
       isDocsHomePage: false,
       permalink: '/docs/foo/bar',
       slug: '/foo/bar',
-      title: 'Bar',
+      title: 'Remarkable',
       description: 'This is custom description',
       frontMatter: {
         description: 'This is custom description',
@@ -255,7 +255,7 @@ describe('simple site', () => {
       isDocsHomePage: true,
       permalink: '/docs/',
       slug: '/',
-      title: 'Bar',
+      title: 'Remarkable',
       description: 'This is custom description',
       frontMatter: {
         description: 'This is custom description',

packages/docusaurus-plugin-content-docs/src/__tests__/index.test.ts

@@ -309,7 +309,7 @@ describe('simple website', () => {
         'foo',
         'bar.md',
       ),
-      title: 'Bar',
+      title: 'Remarkable',
       description: 'This is custom description',
       frontMatter: {
         description: 'This is custom description',

packages/docusaurus-plugin-content-docs/src/docs.ts

@@ -121,9 +121,7 @@ export function processDocMetadata({
     frontMatter: unsafeFrontMatter,
     contentTitle,
     excerpt,
-  } = parseMarkdownString(content, {
-    source,
-  });
+  } = parseMarkdownString(content);
   const frontMatter = validateDocFrontMatter(unsafeFrontMatter);
 
   const {
@@ -205,8 +203,11 @@ export function processDocMetadata({
         numberPrefixParser: options.numberPrefixParser,
       });
 
-  // Default title is the id.
-  const title: string = frontMatter.title ?? contentTitle ?? baseID;
+  // TODO expose both headingTitle+metaTitle to theme?
+  // Different fallbacks order on purpose!
+  // See https://github.com/facebook/docusaurus/issues/4665#issuecomment-825831367
+  const headingTitle: string = contentTitle ?? frontMatter.title ?? baseID;
+  // const metaTitle: string = frontMatter.title ?? contentTitle  ?? baseID;
 
   const description: string = frontMatter.description ?? excerpt ?? '';
 
@@ -245,7 +246,7 @@ export function processDocMetadata({
     unversionedId,
     id,
     isDocsHomePage,
-    title,
+    title: headingTitle,
     description,
     source: aliasedSitePath(filePath, siteDir),
     sourceDirName,

packages/docusaurus-theme-classic/src/theme/DocItem/index.tsx

@@ -24,15 +24,13 @@ import {
 
 function DocItem(props: Props): JSX.Element {
   const {content: DocContent} = props;
+  const {metadata, frontMatter} = DocContent;
   const {
-    metadata,
-    frontMatter: {
     image,
     keywords,
     hide_title: hideTitle,
     hide_table_of_contents: hideTableOfContents,
-    },
-  } = DocContent;
+  } = frontMatter;
   const {
     description,
     title,
@@ -51,9 +49,13 @@ function DocItem(props: Props): JSX.Element {
   // See https://github.com/facebook/docusaurus/issues/3362
   const showVersionBadge = versions.length > 1;
 
+  // For meta title, using frontMatter.title in priority over a potential # title found in markdown
+  // See https://github.com/facebook/docusaurus/issues/4665#issuecomment-825831367
+  const metaTitle = frontMatter.title || title;
+
   return (
     <>
-      <Seo {...{title, description, keywords, image}} />
+      <Seo {...{title: metaTitle, description, keywords, image}} />
 
       <div className="row">
         <div

packages/docusaurus-utils/src/__tests__/markdownParser.test.ts

@@ -138,6 +138,20 @@ describe('parseMarkdownContentTitle', () => {
     });
   });
 
+  test('Should parse markdown h1 title with fixed anchor-id syntax', () => {
+    const markdown = dedent`
+
+          # Markdown Title {#my-anchor-id}
+
+          Lorem Ipsum
+
+        `;
+    expect(parseMarkdownContentTitle(markdown)).toEqual({
+      content: 'Lorem Ipsum',
+      contentTitle: 'Markdown Title',
+    });
+  });
+
   test('Should parse markdown h1 title at the top (atx style with closing #)', () => {
     const markdown = dedent`
 
@@ -152,7 +166,7 @@ describe('parseMarkdownContentTitle', () => {
     });
   });
 
-  test('Should parse markdown h1 title at the top and next one after it', () => {
+  test('Should parse markdown h1 title at the top  followed by h2 title', () => {
     const markdown = dedent`
 
           # Markdown Title
@@ -163,11 +177,61 @@ describe('parseMarkdownContentTitle', () => {
 
         `;
     expect(parseMarkdownContentTitle(markdown)).toEqual({
-      content: '## Heading 2\n\nLorem Ipsum',
+      content: dedent`
+          ## Heading 2
+
+          Lorem Ipsum
+
+        `,
       contentTitle: 'Markdown Title',
     });
   });
 
+  test('Should parse only first h1 title', () => {
+    const markdown = dedent`
+
+          # Markdown Title
+
+          # Markdown Title 2
+
+          Lorem Ipsum
+
+        `;
+    expect(parseMarkdownContentTitle(markdown)).toEqual({
+      content: dedent`
+          # Markdown Title 2
+
+          Lorem Ipsum
+
+        `,
+      contentTitle: 'Markdown Title',
+    });
+  });
+
+  test('Should not parse title that is not at the top', () => {
+    const markdown = dedent`
+
+          Lorem Ipsum
+
+          # Markdown Title 2
+
+          Lorem Ipsum
+
+        `;
+    expect(parseMarkdownContentTitle(markdown)).toEqual({
+      content: dedent`
+
+          Lorem Ipsum
+
+          # Markdown Title 2
+
+          Lorem Ipsum
+
+        `,
+      contentTitle: undefined,
+    });
+  });
+
   test('Should parse markdown h1 alternate title', () => {
     const markdown = dedent`
 
@@ -185,8 +249,10 @@ describe('parseMarkdownContentTitle', () => {
 
   test('Should parse markdown h1 title placed after import declarations', () => {
     const markdown = dedent`
-          import Component from '@site/src/components/Component';
-          import Component from '@site/src/components/Component'
+          import Component1 from '@site/src/components/Component1';
+
+          import Component2 from '@site/src/components/Component2'
+          import Component3 from '@site/src/components/Component3'
           import './styles.css';
 
           # Markdown Title
@@ -194,8 +260,20 @@ describe('parseMarkdownContentTitle', () => {
           Lorem Ipsum
 
         `;
+
+    // remove the useless line breaks? Does not matter too much
     expect(parseMarkdownContentTitle(markdown)).toEqual({
-      content: `import Component from '@site/src/components/Component';\nimport Component from '@site/src/components/Component'\nimport './styles.css';\n\n\n\nLorem Ipsum`,
+      content: dedent`
+          import Component1 from '@site/src/components/Component1';
+
+          import Component2 from '@site/src/components/Component2'
+          import Component3 from '@site/src/components/Component3'
+          import './styles.css';
+
+
+
+          Lorem Ipsum
+        `,
       contentTitle: 'Markdown Title',
     });
   });
@@ -213,7 +291,13 @@ describe('parseMarkdownContentTitle', () => {
 
         `;
     expect(parseMarkdownContentTitle(markdown)).toEqual({
-      content: `import Component from '@site/src/components/Component';\nimport Component from '@site/src/components/Component'\nimport './styles.css';\n\nLorem Ipsum`,
+      content: dedent`
+          import Component from '@site/src/components/Component';
+          import Component from '@site/src/components/Component'
+          import './styles.css';
+
+          Lorem Ipsum
+        `,
       contentTitle: 'Markdown Title',
     });
   });
@@ -277,20 +361,6 @@ describe('parseMarkdownContentTitle', () => {
 });
 
 describe('parseMarkdownString', () => {
-  const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
-  beforeEach(() => {
-    warn.mockReset();
-  });
-
-  function expectDuplicateTitleWarning() {
-    expect(warn).toBeCalledWith(
-      expect.stringMatching(/Duplicate title found in this file/),
-    );
-  }
-  function expectNoWarning() {
-    expect(warn).not.toBeCalled();
-  }
-
   test('parse markdown with frontmatter', () => {
     expect(
       parseMarkdownString(dedent`
@@ -310,7 +380,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectNoWarning();
   });
 
   test('should parse first heading as contentTitle', () => {
@@ -328,7 +397,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should warn about duplicate titles (frontmatter + markdown)', () => {
@@ -352,7 +420,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectDuplicateTitleWarning();
   });
 
   test('should warn about duplicate titles (frontmatter + markdown alternate)', () => {
@@ -377,7 +444,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectDuplicateTitleWarning();
   });
 
   test('should not warn for duplicate title if keepContentTitle=true', () => {
@@ -406,7 +472,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectNoWarning();
   });
 
   test('should not warn for duplicate title if markdown title is not at the top', () => {
@@ -432,7 +497,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectNoWarning();
   });
 
   test('should parse markdown title and keep it in content', () => {
@@ -451,7 +515,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should delete only first heading', () => {
@@ -477,7 +540,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should parse front-matter and ignore h2', () => {
@@ -500,7 +562,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectNoWarning();
   });
 
   test('should read front matter only', () => {
@@ -520,7 +581,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectNoWarning();
   });
 
   test('should parse title only', () => {
@@ -532,7 +592,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should parse title only alternate', () => {
@@ -549,7 +608,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should warn about duplicate titles', () => {
@@ -570,7 +628,6 @@ describe('parseMarkdownString', () => {
         },
       }
     `);
-    expectDuplicateTitleWarning();
   });
 
   test('should ignore markdown title if its not a first text', () => {
@@ -588,7 +645,6 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 
   test('should delete only first heading', () => {
@@ -614,6 +670,5 @@ describe('parseMarkdownString', () => {
         "frontMatter": Object {},
       }
     `);
-    expectNoWarning();
   });
 });

packages/docusaurus-utils/src/markdownParser.ts

@@ -86,7 +86,7 @@ export function parseMarkdownContentTitle(
 
   const content = contentUntrimmed.trim();
 
-  const regularTitleMatch = /^(?:import\s.*(from.*)?;?|\n)*?(?<pattern>#\s*(?<title>[^#\n]*)+[ \t]*#?\n*?)/g.exec(
+  const regularTitleMatch = /^(?:import\s.*(from.*)?;?|\n)*?(?<pattern>#\s*(?<title>[^#\n{]*)+[ \t]*(?<suffix>({#*[\w-]+})|#)?\n*?)/g.exec(
     content,
   );
   const alternateTitleMatch = /^(?:import\s.*(from.*)?;?|\n)*?(?<pattern>\s*(?<title>[^\n]*)\s*\n[=]+)/g.exec(
@@ -120,12 +120,10 @@ type ParsedMarkdown = {
 export function parseMarkdownString(
   markdownFileContent: string,
   options?: {
-    source?: string;
     keepContentTitle?: boolean;
   },
 ): ParsedMarkdown {
   try {
-    const sourceOption = options?.source;
     const keepContentTitle = options?.keepContentTitle ?? false;
 
     const {frontMatter, content: contentWithoutFrontMatter} = parseFrontMatter(
@@ -141,20 +139,6 @@ export function parseMarkdownString(
 
     const excerpt = createExcerpt(content);
 
-    // TODO not sure this is a good place for this warning
-    if (
-      frontMatter.title &&
-      contentTitle &&
-      !keepContentTitle &&
-      !(process.env.DOCUSAURUS_NO_DUPLICATE_TITLE_WARNING === 'false')
-    ) {
-      console.warn(
-        chalk.yellow(`Duplicate title found in ${sourceOption ?? 'this'} file.
-Use either a frontmatter title or a markdown title, not both.
-If this is annoying you, use env DOCUSAURUS_NO_DUPLICATE_TITLE_WARNING=false`),
-      );
-    }
-
     return {
       frontMatter,
       content,
@@ -175,7 +159,7 @@ export async function parseMarkdownFile(
 ): Promise<ParsedMarkdown> {
   const markdownString = await fs.readFile(source, 'utf-8');
   try {
-    return parseMarkdownString(markdownString, {source});
+    return parseMarkdownString(markdownString);
   } catch (e) {
     throw new Error(
       `Error while parsing markdown file ${source}

packages/docusaurus/src/commands/__tests__/writeHeadingIds.test.ts

@@ -72,10 +72,12 @@ describe('transformMarkdownContent', () => {
   test('transform the headings', () => {
     const input = `
 
-# Hello world
+# Ignorerd title
 
 ## abc
 
+### Hello world
+
 \`\`\`
 # Heading in code block
 \`\`\`
@@ -95,10 +97,12 @@ describe('transformMarkdownContent', () => {
     // not sure how to implement that atm
     const expected = `
 
-# Hello world {#hello-world}
+# Ignorerd title
 
 ## abc {#abc}
 
+### Hello world {#hello-world}
+
 \`\`\`
 # Heading in code block
 \`\`\`

packages/docusaurus/src/commands/writeHeadingIds.ts

@@ -53,7 +53,8 @@ export function transformMarkdownLine(
   line: string,
   slugger: GithubSlugger,
 ): string {
-  if (line.startsWith('#')) {
+  // Ignore h1 headings on purpose, as we don't create anchor links for those
+  if (line.startsWith('##')) {
     return transformMarkdownHeadingLine(line, slugger);
   } else {
     return line;