fix(docs): prevent docs ids conflicts within a version (#11251)

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/frontMatter/deeply/doc.md

@@ -0,0 +1,4 @@
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/frontMatter/deeply/nested/whatever.md

@@ -0,0 +1,7 @@
+---
+id: doc
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/frontMatter/doc.md

@@ -0,0 +1,7 @@
+---
+# no id but should conflict due to the name anyway
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/frontMatter/doc1.md

@@ -0,0 +1,7 @@
+---
+id: doc
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/frontMatter/doc2.md

@@ -0,0 +1,7 @@
+---
+id: doc
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/1-doc.md

@@ -0,0 +1,3 @@
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/2-doc.md

@@ -0,0 +1,7 @@
+---
+id: doc
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/deeply/1-doc.md

@@ -0,0 +1,3 @@
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/deeply/nested/2-doc.md

@@ -0,0 +1,3 @@
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/deeply/nested/3-doc.md

@@ -0,0 +1,3 @@
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/distinct-frontMatter-id/1-doc.md

@@ -0,0 +1,7 @@
+---
+id: doc-1
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/distinct-frontMatter-id/2-doc.md

@@ -0,0 +1,7 @@
+---
+id: doc-2
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versioned_docs/version-with-id-conflicts/number-prefix/distinct-frontMatter-id/3-doc.md

@@ -0,0 +1,7 @@
+---
+id: doc-3
+---
+
+# Hello
+
+World

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-broken-versions/versions.json

@@ -0,0 +1,3 @@
+[
+  "with-id-conflicts"
+]

packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-minimal/docs/hello.md

@@ -0,0 +1,4 @@
+
+# Hello
+
+World

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

@@ -1,12 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`minimal site can load current version 1`] = `
+exports[`loadVersion minimal site can load current version 1`] = `
 {
   "badge": false,
   "banner": null,
   "className": "docs-version-current",
-  "contentPath": "<PROJECT_ROOT>/packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/minimal-site/docs",
+  "contentPath": "<PROJECT_ROOT>/packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-minimal/docs",
-  "contentPathLocalized": "<PROJECT_ROOT>/packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/minimal-site/i18n/en/docusaurus-plugin-content-docs/current",
+  "contentPathLocalized": "<PROJECT_ROOT>/packages/docusaurus-plugin-content-docs/src/versions/__tests__/__fixtures__/site-minimal/i18n/en/docusaurus-plugin-content-docs/current",
   "docs": [
     {
       "description": "World",

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

@@ -22,52 +22,96 @@ const DefaultI18N: I18n = {
   localeConfigs: {},
 };
 
-describe('minimal site', () => {
+async function siteFixture(fixture: string) {
-  async function loadSite() {
+  const siteDir = path.resolve(path.join(__dirname, './__fixtures__', fixture));
-    const siteDir = path.resolve(
+  const options: PluginOptions = fromPartial<PluginOptions>({
-      path.join(__dirname, './__fixtures__', 'minimal-site'),
+    id: 'default',
-    );
+    ...DEFAULT_OPTIONS,
-    const options: PluginOptions = fromPartial<PluginOptions>({
+  });
-      ...DEFAULT_OPTIONS,
+  const context = fromPartial<LoadContext>({
-    });
+    siteDir,
-    const context = fromPartial<LoadContext>({
+    baseUrl: '/',
-      siteDir,
+    i18n: DefaultI18N,
-      baseUrl: '/',
+    localizationDir: path.join(siteDir, 'i18n/en'),
-      i18n: DefaultI18N,
+    siteConfig: {
-      localizationDir: path.join(siteDir, 'i18n/en'),
+      markdown: {
-      siteConfig: {
+        parseFrontMatter: DEFAULT_PARSE_FRONT_MATTER,
-        markdown: {
-          parseFrontMatter: DEFAULT_PARSE_FRONT_MATTER,
-        },
       },
+    },
+  });
+
+  const versions = await readVersionsMetadata({
+    options,
+    context,
+  });
+
+  return {
+    siteDir,
+    options,
+    context,
+    versions,
+  };
+}
+
+describe('loadVersion', () => {
+  describe('minimal site', () => {
+    it('can load current version', async () => {
+      const {options, context, versions} = await siteFixture('site-minimal');
+
+      const version = versions[0];
+      expect(version).toBeDefined();
+      expect(version.versionName).toBe('current');
+
+      const loadedVersion = loadVersion({
+        context,
+        options,
+        versionMetadata: version,
+        env: 'production',
+      });
+
+      await expect(loadedVersion).resolves.toMatchSnapshot();
     });
-    return {
+  });
-      siteDir,
-      options,
-      context,
-    };
-  }
 
-  it('can load current version', async () => {
+  describe('site with broken versions', () => {
-    const {options, context} = await loadSite();
+    async function loadTestVersion(versionName: string) {
+      const {options, context, versions} = await siteFixture(
+        'site-broken-versions',
+      );
+      const version = versions.find((v) => v.versionName === versionName);
+      if (!version) {
+        throw new Error(`Version '${versionName}' should exist`);
+      }
+      return loadVersion({
+        context,
+        options,
+        versionMetadata: version,
+        env: 'production',
+      });
+    }
 
-    const versionsMetadata = await readVersionsMetadata({
+    it('rejects version with doc id conflict', async () => {
-      options,
+      await expect(() => loadTestVersion('with-id-conflicts')).rejects
-      context,
+        .toThrowErrorMatchingInlineSnapshot(`
+        "The docs plugin found docs sharing the same id:
+
+        - \`frontMatter/doc\` found in 3 docs:
+          - versioned_docs/version-with-id-conflicts/frontMatter/doc.md
+          - versioned_docs/version-with-id-conflicts/frontMatter/doc1.md
+          - versioned_docs/version-with-id-conflicts/frontMatter/doc2.md
+
+        - \`number-prefix/doc\` found in 2 docs:
+          - versioned_docs/version-with-id-conflicts/number-prefix/1-doc.md
+          - versioned_docs/version-with-id-conflicts/number-prefix/2-doc.md
+
+        - \`number-prefix/deeply/nested/doc\` found in 2 docs:
+          - versioned_docs/version-with-id-conflicts/number-prefix/deeply/nested/2-doc.md
+          - versioned_docs/version-with-id-conflicts/number-prefix/deeply/nested/3-doc.md
+
+        Docs should have distinct ids.
+        In case of conflict, you can rename the docs file, or use the \`id\` front matter to assign an explicit distinct id to each doc.
+            "
+      `);
     });
-
-    expect(versionsMetadata).toHaveLength(1);
-    expect(versionsMetadata[0]!.versionName).toBe('current');
-
-    const versionMetadata = versionsMetadata[0]!;
-
-    const loadedVersion = loadVersion({
-      context,
-      options,
-      versionMetadata,
-      env: 'production',
-    });
-
-    await expect(loadedVersion).resolves.toMatchSnapshot();
   });
 });

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

@@ -7,7 +7,7 @@
 
 import path from 'path';
 import _ from 'lodash';
-import {createSlugger} from '@docusaurus/utils';
+import {aliasedSitePathToRelativePath, createSlugger} from '@docusaurus/utils';
 import {getTagsFile} from '@docusaurus/utils-validation';
 import logger from '@docusaurus/logger';
 import {
@@ -29,102 +29,151 @@ import type {
 import type {DocFile} from '../types';
 import type {LoadContext} from '@docusaurus/types';
 
-export async function loadVersion({
+type LoadVersionParams = {
-  context,
-  options,
-  versionMetadata,
-  env,
-}: {
   context: LoadContext;
   options: PluginOptions;
   versionMetadata: VersionMetadata;
   env: DocEnv;
-}): Promise<LoadedVersion> {
+};
-  const {siteDir} = context;
 
-  async function loadVersionDocsBase(
+function ensureNoDuplicateDocId(docs: DocMetadataBase[]): void {
-    tagsFile: TagsFile | null,
+  const duplicatesById = _.chain(docs)
-  ): Promise<DocMetadataBase[]> {
+    .groupBy((d) => d.id)
-    const docFiles = await readVersionDocs(versionMetadata, options);
+    .pickBy((group) => group.length > 1)
-    if (docFiles.length === 0) {
+    .value();
-      throw new Error(
+
-        `Docs version "${
+  const duplicateIdEntries = Object.entries(duplicatesById);
-          versionMetadata.versionName
+
-        }" has no docs! At least one doc should exist at "${path.relative(
+  if (duplicateIdEntries.length) {
-          siteDir,
+    const idMessages = duplicateIdEntries
-          versionMetadata.contentPath,
+      .map(([id, duplicateDocs]) => {
-        )}".`,
+        return logger.interpolate`- code=${id} found in number=${
-      );
+          duplicateDocs.length
-    }
+        } docs:
-    function processVersionDoc(docFile: DocFile) {
+  - ${duplicateDocs
-      return processDocMetadata({
+    .map((d) => aliasedSitePathToRelativePath(d.source))
-        docFile,
+    .join('\n  - ')}`;
-        versionMetadata,
+      })
-        context,
+      .join('\n\n');
-        options,
+
-        env,
+    const message = `The docs plugin found docs sharing the same id:
-        tagsFile,
+\n${idMessages}\n
-      });
+Docs should have distinct ids.
-    }
+In case of conflict, you can rename the docs file, or use the ${logger.code(
-    return Promise.all(docFiles.map(processVersionDoc));
+      'id',
+    )} front matter to assign an explicit distinct id to each doc.
+    `;
+
+    throw new Error(message);
   }
+}
 
-  async function doLoadVersion(): Promise<LoadedVersion> {
+async function loadVersionDocsBase({
-    const tagsFile = await getTagsFile({
+  tagsFile,
-      contentPaths: versionMetadata,
+  context,
-      tags: options.tags,
+  options,
+  versionMetadata,
+  env,
+}: LoadVersionParams & {
+  tagsFile: TagsFile | null;
+}): Promise<DocMetadataBase[]> {
+  const docFiles = await readVersionDocs(versionMetadata, options);
+  if (docFiles.length === 0) {
+    throw new Error(
+      `Docs version "${
+        versionMetadata.versionName
+      }" has no docs! At least one doc should exist at "${path.relative(
+        context.siteDir,
+        versionMetadata.contentPath,
+      )}".`,
+    );
+  }
+  function processVersionDoc(docFile: DocFile) {
+    return processDocMetadata({
+      docFile,
+      versionMetadata,
+      context,
+      options,
+      env,
+      tagsFile,
     });
+  }
+  const docs = await Promise.all(docFiles.map(processVersionDoc));
+  ensureNoDuplicateDocId(docs);
+  return docs;
+}
 
-    const docsBase: DocMetadataBase[] = await loadVersionDocsBase(tagsFile);
+async function doLoadVersion({
+  context,
+  options,
+  versionMetadata,
+  env,
+}: LoadVersionParams): Promise<LoadedVersion> {
+  const tagsFile = await getTagsFile({
+    contentPaths: versionMetadata,
+    tags: options.tags,
+  });
 
-    // TODO we only ever need draftIds in further code, not full draft items
+  const docsBase: DocMetadataBase[] = await loadVersionDocsBase({
-    // To simplify and prevent mistakes, avoid exposing draft
+    tagsFile,
-    // replace draft=>draftIds in content loaded
+    context,
-    const [drafts, docs] = _.partition(docsBase, (doc) => doc.draft);
+    options,
+    versionMetadata,
+    env,
+  });
 
-    const sidebars = await loadSidebars(versionMetadata.sidebarFilePath, {
+  // TODO we only ever need draftIds in further code, not full draft items
-      sidebarItemsGenerator: options.sidebarItemsGenerator,
+  // To simplify and prevent mistakes, avoid exposing draft
-      numberPrefixParser: options.numberPrefixParser,
+  // replace draft=>draftIds in content loaded
+  const [drafts, docs] = _.partition(docsBase, (doc) => doc.draft);
+
+  const sidebars = await loadSidebars(versionMetadata.sidebarFilePath, {
+    sidebarItemsGenerator: options.sidebarItemsGenerator,
+    numberPrefixParser: options.numberPrefixParser,
+    docs,
+    drafts,
+    version: versionMetadata,
+    sidebarOptions: {
+      sidebarCollapsed: options.sidebarCollapsed,
+      sidebarCollapsible: options.sidebarCollapsible,
+    },
+    categoryLabelSlugger: createSlugger(),
+  });
+
+  const sidebarsUtils = createSidebarsUtils(sidebars);
+
+  const docsById = createDocsByIdIndex(docs);
+  const allDocIds = Object.keys(docsById);
+
+  sidebarsUtils.checkLegacyVersionedSidebarNames({
+    sidebarFilePath: versionMetadata.sidebarFilePath as string,
+    versionMetadata,
+  });
+  sidebarsUtils.checkSidebarsDocIds({
+    allDocIds,
+    sidebarFilePath: versionMetadata.sidebarFilePath as string,
+    versionMetadata,
+  });
+
+  return {
+    ...versionMetadata,
+    docs: addDocNavigation({
       docs,
-      drafts,
+      sidebarsUtils,
-      version: versionMetadata,
+    }),
-      sidebarOptions: {
+    drafts,
-        sidebarCollapsed: options.sidebarCollapsed,
+    sidebars,
-        sidebarCollapsible: options.sidebarCollapsible,
+  };
-      },
+}
-      categoryLabelSlugger: createSlugger(),
-    });
-
-    const sidebarsUtils = createSidebarsUtils(sidebars);
-
-    const docsById = createDocsByIdIndex(docs);
-    const allDocIds = Object.keys(docsById);
-
-    sidebarsUtils.checkLegacyVersionedSidebarNames({
-      sidebarFilePath: versionMetadata.sidebarFilePath as string,
-      versionMetadata,
-    });
-    sidebarsUtils.checkSidebarsDocIds({
-      allDocIds,
-      sidebarFilePath: versionMetadata.sidebarFilePath as string,
-      versionMetadata,
-    });
-
-    return {
-      ...versionMetadata,
-      docs: addDocNavigation({
-        docs,
-        sidebarsUtils,
-      }),
-      drafts,
-      sidebars,
-    };
-  }
 
+export async function loadVersion(
+  params: LoadVersionParams,
+): Promise<LoadedVersion> {
   try {
-    return await doLoadVersion();
+    return await doLoadVersion(params);
   } catch (err) {
-    logger.error`Loading of version failed for version name=${versionMetadata.versionName}`;
+    // TODO use error cause (but need to refactor many tests)
+    logger.error`Loading of version failed for version name=${params.versionMetadata.versionName}`;
     throw err;
   }
 }

website/_dogfooding/_docs tests/tests/Conflicts/file-name/1-file-name.mdx

@@ -0,0 +1,8 @@
+---
+id: file-name-1
+slug: file-name-1
+---
+
+# File name 1
+
+File name 1

website/_dogfooding/_docs tests/tests/Conflicts/file-name/2-file-name.mdx

@@ -0,0 +1,8 @@
+---
+id: file-name-2
+slug: file-name-2
+---
+
+# File name 2
+
+File name 2

website/_dogfooding/_docs tests/tests/Conflicts/file-name/_category_.json

@@ -0,0 +1,8 @@
+{
+  "label": "File name conflict",
+  "link": {
+    "type": "generated-index",
+    "title": "File name conflict",
+    "description": "Testing what happens when 2 files have the same name but different position prefixes"
+  }
+}