fix(API): patch race conditions, apiIndex dual-producer conflict

cli/__tests__/createCollectionContent.test.ts

@@ -1,4 +1,7 @@
-import { createCollectionContent } from '../createCollectionContent'
+import {
+  createCollectionContent,
+  resolveContentEntryGlob,
+} from '../createCollectionContent'
 import { getConfig } from '../getConfig'
 import { writeFile } from 'fs/promises'
 import { existsSync } from 'fs'
@@ -14,6 +17,64 @@ beforeEach(() => {
   jest.clearAllMocks()
 })
 
+describe('resolveContentEntryGlob', () => {
+  it('anchors a local content glob to the config directory', () => {
+    ;(existsSync as jest.Mock).mockReturnValue(true)
+
+    expect(
+      resolveContentEntryGlob(
+        { name: 'local-docs', base: 'docs', pattern: '**/*.md' },
+        '/config/dir',
+        '/config',
+      ),
+    ).toBe('/config/dir/docs/**/*.md')
+  })
+
+  it('returns null for an unavailable local content directory', () => {
+    ;(existsSync as jest.Mock).mockReturnValue(false)
+
+    expect(
+      resolveContentEntryGlob(
+        { name: 'local-docs', base: 'missing-docs', pattern: '**/*.md' },
+        '/config/dir',
+        '/config',
+      ),
+    ).toBeNull()
+  })
+
+  it('resolves a package content glob from node_modules', () => {
+    ;(existsSync as jest.Mock).mockReturnValue(true)
+
+    expect(
+      resolveContentEntryGlob(
+        {
+          name: 'component-docs',
+          packageName: '@patternfly/react-core',
+          pattern: '**/*.md',
+        },
+        '/config/dir',
+        '/config/dir',
+      ),
+    ).toBe('/config/dir/node_modules/@patternfly/react-core/**/*.md')
+  })
+
+  it('returns null for an unavailable package', () => {
+    ;(existsSync as jest.Mock).mockReturnValue(false)
+
+    expect(
+      resolveContentEntryGlob(
+        {
+          name: 'component-docs',
+          packageName: '@patternfly/react-core',
+          pattern: '**/*.md',
+        },
+        '/config/dir',
+        '/config/dir',
+      ),
+    ).toBeNull()
+  })
+})
+
 it('should call getConfig with the passed config file location', async () => {
   jest.spyOn(console, 'error').mockImplementation(() => {})
   ;(getConfig as jest.Mock).mockResolvedValue(undefined)

cli/cli.ts

@@ -3,7 +3,10 @@
 import { Command } from 'commander'
 import { build, dev, preview, sync } from 'astro'
 import { dirname, join, resolve } from 'path'
-import { createCollectionContent } from './createCollectionContent.js'
+import {
+  createCollectionContent,
+  resolveContentEntryGlob,
+} from './createCollectionContent.js'
 import { setFsRootDir } from './setFsRootDir.js'
 import { createConfigFile } from './createConfigFile.js'
 import { updatePackageFile } from './updatePackageFile.js'
@@ -87,12 +90,34 @@ async function generateProps(program: Command, forceProps: boolean = false) {
   await buildPropsData(rootDir, `${currentDir}/pf-docs.config.mjs`, verbose)
 }
 
-async function transformMDContentToMDX() {
-  if (config.content) {
-    await Promise.all(
-      config.content.map((contentObj) => convertToMDX(contentObj.pattern)),
-    )
+async function transformMDContentToMDX(verbose = false) {
+  if (!config.content) {
+    return
   }
+
+  const repoRootDir = config.repoRoot
+    ? resolve(currentDir, config.repoRoot)
+    : currentDir
+
+  await Promise.all(
+    config.content.map(async (contentObj) => {
+      const globPath = resolveContentEntryGlob(
+        contentObj,
+        currentDir,
+        repoRootDir,
+        verbose,
+      )
+
+      if (!globPath) {
+        throw new Error(
+          `Unable to resolve documentation content "${contentObj.name}". ` +
+            'Check its base or packageName configuration.',
+        )
+      }
+
+      await convertToMDX(globPath)
+    }),
+  )
 }
 
 async function initializeApiIndex(program: Command) {
@@ -140,9 +165,9 @@ async function buildProject(program: Command): Promise<DocsConfig | undefined> {
     return config
   }
   await updateContent(program)
+  await transformMDContentToMDX(verbose)
   await generateProps(program, true)
   await initializeApiIndex(program)
-  await transformMDContentToMDX()
 
   const docsOutputDir = join(absoluteOutputDir, 'docs')
 
@@ -151,20 +176,6 @@ async function buildProject(program: Command): Promise<DocsConfig | undefined> {
     outDir: docsOutputDir,
   })
 
-  // copy the apiIndex.json file to the docs directory so it can be served as a static asset
-  try {
-    const apiIndexPath = join(absoluteOutputDir, 'apiIndex.json')
-    const docsApiIndexPath = join(absoluteOutputDir, 'docs', 'apiIndex.json')
-    await copyFile(apiIndexPath, docsApiIndexPath)
-
-    if (verbose) {
-      console.log('Copied apiIndex.json to docs directory')
-    }
-  } catch (error) {
-    console.error('Failed to copy apiIndex.json to docs directory:', error)
-    throw error
-  }
-
   return config
 }
 
@@ -224,11 +235,12 @@ program.command('init').action(async () => {
 })
 
 program.command('start').action(async () => {
-  const { apiOnly } = program.opts()
+  const { apiOnly, verbose } = program.opts()
   if (apiOnly) {
     process.env.PF_API_ONLY = 'true'
   }
   await updateContent(program)
+  await transformMDContentToMDX(verbose)
   await initializeApiIndex(program)
 
   // if a props file hasn't been generated yet, but the consumer has propsData, it will cause a runtime error so to

cli/createCollectionContent.ts

@@ -1,20 +1,20 @@
 /* eslint-disable no-console */
 import { writeFile } from 'fs/promises'
 import { dirname, join, resolve } from 'path'
-import { getConfig } from './getConfig.js'
+import { getConfig, type CollectionDefinition } from './getConfig.js'
 import { existsSync } from 'fs'
 
 /**
  * Looks for the specified package in the node_modules directory in the configDir, then recursively
  * looks for the package in the parent directories until it is found, stopping if it is not found
  * and the repoRootDir is reached.
  */
-const findPackage = (
+export const findPackage = (
   dir: string,
   packageName: string,
   repoRootDir: string,
   verbose?: boolean,
-) => {
+): string | null => {
   const nodeModulesPath = join(dir, 'node_modules')
 
   if (verbose) {
@@ -42,6 +42,33 @@ const findPackage = (
   return findPackage(parentDir, packageName, repoRootDir, verbose)
 }
 
+export function resolveContentEntryGlob(
+  contentEntry: CollectionDefinition,
+  configDir: string,
+  repoRootDir: string,
+  verbose?: boolean,
+): string | null {
+  if (contentEntry.base) {
+    const basePath = resolve(configDir, contentEntry.base)
+    return existsSync(basePath)
+      ? join(basePath, contentEntry.pattern)
+      : null
+  }
+
+  if (!contentEntry.packageName) {
+    return null
+  }
+
+  const packagePath = findPackage(
+    configDir,
+    contentEntry.packageName,
+    repoRootDir,
+    verbose,
+  )
+
+  return packagePath ? join(packagePath, contentEntry.pattern) : null
+}
+
 export async function createCollectionContent(
   astroRoot: string,
   configFileLocation: string,

package.json

@@ -10,9 +10,9 @@
     "dev": "npm run start:cli",
     "start": "npm run dev",
     "start:cli": "npm run build:cli && node ./dist/cli/cli.js start",
-    "start:astro": "astro dev",
+    "start:astro": "npm run start:cli",
     "build": "npm run build:cli && node --max-old-space-size=8192 ./dist/cli/cli.js build",
-    "build:astro": "astro check && astro build",
+    "build:astro": "npm run build && astro check",
     "build:cli": "tsc --build ./cli/tsconfig.json",
     "build:cli:watch": "tsc --build --watch ./cli/tsconfig.json",
     "build:props": "npm run build:cli && node ./dist/cli/cli.js generate-props",

src/content.config.ts

@@ -3,7 +3,6 @@ import { glob } from 'astro/loaders'
 
 import { content } from './content'
 import type { CollectionDefinition } from '../cli/getConfig'
-import { convertToMDX } from '../cli/convertToMDX'
 
 function defineContent(contentObj: CollectionDefinition) {
   const { base, packageName, pattern, name, frontmatterDefaults, frontmatterMapping } = contentObj
@@ -20,7 +19,7 @@ function defineContent(contentObj: CollectionDefinition) {
     'core-component-docs': 'html',
   }
 
-  convertToMDX(`${base}/${pattern}`)
+  // Conversion is completed by the CLI before Astro loads collections.
   const mdxPattern = pattern.replace(/\.md$/, '.mdx')
 
   const hasExternalFrontmatter = !!(frontmatterDefaults || frontmatterMapping)

src/pages/apiIndex.json.ts

@@ -1,12 +1,12 @@
 import type { APIRoute } from 'astro'
-import { getApiIndex } from '../utils/apiIndex/get'
+import { generateApiIndex } from '../utils/apiIndex/generate'
 
 // Prerender at build time so this doesn't run in the Cloudflare Worker
 export const prerender = true
 
 export const GET: APIRoute = async () => {
   try {
-    const index = await getApiIndex()
+    const index = await generateApiIndex()
     return new Response(JSON.stringify(index), {
       status: 200,
       headers: {

src/utils/__tests__/componentIndex.test.ts

@@ -1,9 +1,16 @@
-import { readFile } from 'fs/promises'
+import { readFile, writeFile } from 'fs/promises'
 import { getCollection } from 'astro:content'
 import { GET } from '../../pages/api/component-index.json'
-import { generateApiIndex } from '../apiIndex/generate'
+import {
+  generateAndWriteApiIndex,
+  generateApiIndex,
+} from '../apiIndex/generate'
 
-jest.mock('fs/promises', () => ({ readFile: jest.fn() }))
+jest.mock('fs/promises', () => ({
+  readFile: jest.fn(),
+  writeFile: jest.fn(),
+  mkdir: jest.fn(),
+}))
 jest.mock('astro:content', () => ({ getCollection: jest.fn() }))
 jest.mock('../../content', () => ({ content: [{ name: 'react-component-docs', version: 'v6' }] }))
 jest.mock('../getOutputDir', () => ({ getOutputDir: jest.fn().mockResolvedValue('/output') }))
@@ -21,7 +28,7 @@ beforeEach(() => {
     { data: { section: 'components', id: 'Chip', tab: 'react-deprecated', propComponents: ['Chip', 'ChipGroup'] }, body: '', filePath: '/deprecated/components/Chip.md' },
     { data: { section: 'components', id: 'Table', tab: 'react', propComponents: ['Table'] }, body: '', filePath: '/components/Table.md' },
     { data: { section: 'extensions', subsection: 'data-view', id: 'Table', tab: 'react', propComponents: ['DataViewTable'] }, body: '', filePath: '/extensions/DataViewTable.md' },
-    { data: { section: 'foundations-and-styles', subsection: 'layouts', id: 'Flex', tab: 'react', propComponents: ['Flex', 'FlexItem'] }, body: '<LiveExample src={FlexBasic} />', filePath: '/layouts/Flex.md' },
+    { data: { section: 'foundations-and-styles', subsection: 'layouts', id: 'Flex', tab: 'react', propComponents: ['Flex', 'FlexItem'] }, body: '### Basic flex\n<LiveExample src={FlexBasic} />', filePath: '/layouts/Flex.md' },
   ])
   ;(readFile as jest.Mock).mockResolvedValue(JSON.stringify(Object.fromEntries(
     ['Nav', 'NavItem', 'FileUpload', 'FileUploadField', 'ActionGroup', 'Form', 'Flex', 'FlexItem', 'Chip-deprecated', 'ChipGroup-deprecated', 'Table', 'DataViewTable'].map((name) => [name, { name, props: [] }]),
@@ -35,7 +42,9 @@ it('retains active frontmatter mappings without HTML or deprecated entries overw
   expect(index.propComponents?.['v6::components::navigation']).toEqual(['Nav', 'NavItem', 'NavMissingProps'])
   expect(index.propComponents?.['v6::components::chip']).toEqual(['Chip', 'ChipGroup'])
   expect(index.propComponents?.['v6::foundations-and-styles::layouts_flex']).toEqual(['Flex', 'FlexItem'])
-  expect(index.examples['v6::foundations-and-styles::layouts_flex::react']).toEqual([{ exampleName: 'FlexBasic', title: null }])
+  expect(index.examples['v6::foundations-and-styles::layouts_flex::react']).toEqual([
+    { exampleName: 'FlexBasic', title: 'Basic flex' },
+  ])
 })
 
 it('indexes current collections, preserving page keys and exposing documented members', async () => {
@@ -58,6 +67,16 @@ it('indexes current collections, preserving page keys and exposing documented me
   expect(readFile).toHaveBeenCalledWith('/output/props.json', 'utf-8')
 })
 
+it('coalesces concurrent API index writes', async () => {
+  const [firstIndex, secondIndex] = await Promise.all([
+    generateAndWriteApiIndex(),
+    generateAndWriteApiIndex(),
+  ])
+
+  expect(firstIndex).toEqual(secondIndex)
+  expect(writeFile).toHaveBeenCalledTimes(1)
+})
+
 it('only advertises deprecated props when the suffixed record exists', async () => {
   ;(readFile as jest.Mock).mockResolvedValue(JSON.stringify({
     Chip: { name: 'Chip', props: [] },

src/utils/apiIndex/generate.ts

@@ -270,7 +270,8 @@ export async function writeApiIndex(index: ApiIndex): Promise<void> {
     await writeFile(indexPath, JSON.stringify(index, null, 2))
     console.log(`✓ Generated API index with ${index.versions.length} versions`)
   } catch (error) {
-    console.warn('Warning: Could not write API index file:', error)
+    console.error('Could not write API index file:', error)
+    throw error
   }
 }
 
@@ -280,8 +281,21 @@ export async function writeApiIndex(index: ApiIndex): Promise<void> {
  *
  * @returns Promise resolving to the generated API index
  */
+let indexWriteInFlight: Promise<ApiIndex> | null = null
+
 export async function generateAndWriteApiIndex(): Promise<ApiIndex> {
-  const index = await generateApiIndex()
-  await writeApiIndex(index)
-  return index
+  if (indexWriteInFlight) {
+    return indexWriteInFlight
+  }
+
+  indexWriteInFlight = generateApiIndex()
+    .then(async (index) => {
+      await writeApiIndex(index)
+      return index
+    })
+    .finally(() => {
+      indexWriteInFlight = null
+    })
+
+  return indexWriteInFlight
 }