From 41cf338ed5584bbd0839d9ebc26faad2adba06ea Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Sat, 24 Oct 2026 10:19:09 -0400 Subject: [PATCH 1/2] fix(API): prevent MDX example import collisions * cli/convertToMDX, replace the broad import matcher with explicit handling for absolute non-CSS default, named, compound, multiline, and type-only imports before generating LiveExample raw imports * cli/convertToMDX, preserve relative and CSS imports so local assets and stylesheet imports remain available to MDX * cli/__tests__/convertToMDX, add import-shape coverage for default, named, aliased, compound, multiline, type-only, relative, and CSS imports * cli/__tests__/convertToMDX, add the Chatbot regression case proving compound imports are removed before Chatbot and ChatbotDisplayMode raw-example imports are generated * tests, document current fenced-code and namespace-import limitations as intentional scope boundaries for this surgical fix Assisted-by: Cursor-auto, GPT-5, Claude-Fable-5 --- cli/__tests__/convertToMDX.test.ts | 123 +++++++++++++++++++++++++++++ cli/convertToMDX.ts | 22 +++++- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/cli/__tests__/convertToMDX.test.ts b/cli/__tests__/convertToMDX.test.ts index 1604b56..0f2777a 100644 --- a/cli/__tests__/convertToMDX.test.ts +++ b/cli/__tests__/convertToMDX.test.ts @@ -113,6 +113,129 @@ it('reuses repeated imports and disambiguates filenames in different directories expect(converted).toContain('') }) +it.each([ + // [description, importLine, shouldBeRemoved] + ['default absolute import', "import Chatbot from '@patternfly/chatbot/dist/dynamic/Chatbot'", true], + ['named absolute import', "import { Foo } from '@patternfly/react-core'", true], + ['aliased named absolute import', "import { Foo as Bar } from '@patternfly/react-core'", true], + ['multiple named absolute imports', "import { Foo, Bar } from '@patternfly/react-core'", true], + ['compound default and named absolute import', "import Chatbot, { ChatbotDisplayMode } from '@patternfly/chatbot/dist/dynamic/Chatbot'", true], + ['semicolon-terminated absolute import', "import Chatbot from '@patternfly/chatbot/dist/dynamic/Chatbot';", true], + ['default type-only absolute import', "import type Foo from '@patternfly/some-package'", true], + ['named type-only absolute import', "import type { Bar } from '@patternfly/some-package'", true], + ['relative import', "import Kept from './Kept'", false], + ['parent-relative import', "import Kept from '../Kept'", false], + ['relative named import', "import { Kept } from './Kept'", false], + ['absolute CSS import', "import '@patternfly/react-core/dist/styles/base.css'", false], +])('import removal: %s', async (_desc, importLine, shouldBeRemoved) => { + ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) + ;(readFile as jest.Mock).mockResolvedValue(`${importLine}\n# Heading`) + + await convertToMDX('test.md') + + const converted = (writeFile as jest.Mock).mock.calls[0][1] + if (shouldBeRemoved) { + expect(converted).not.toContain(importLine) + } else { + expect(converted).toContain(importLine) + } +}) + +it.each<[string, string, (converted: string) => void]>([ + // [limitation, input, assertion of CURRENT (imperfect) behavior] + // These rows document accepted gaps, not desired behavior. None are reachable in the + // currently audited docs. Fixing most of them requires fence-masking (running the + // transforms only on unfenced segments); flipping a row is a deliberate contract change. + [ + 'absolute imports inside code fences are stripped, corrupting the sample', + "```js\nimport { Button } from '@patternfly/react-core';\nconst x = 1;\n```", + (converted) => { + expect(converted).not.toContain("import { Button } from '@patternfly/react-core';") + expect(converted).toContain('const x = 1;') + }, + ], + [ + 'empty file= fences nested inside outer demo fences are still converted', + '````md\n```ts file="./Example.tsx"\n```\n````', + (converted) => { + expect(converted).toContain('import Example from "./Example.tsx?raw"') + expect(converted).toContain('') + }, + ], + [ + 'HTML comments inside code fences are rewritten to MDX comments', + '```\n\n```', + (converted) => expect(converted).toContain('{/* note */}'), + ], + [ + 'namespace absolute imports are preserved', + "import * as React from 'react'\n# Heading", + (converted) => expect(converted).toContain("import * as React from 'react'"), + ], +])('known limitation (documents current behavior; changing it is a deliberate decision): %s', async (_desc, input, assertCurrentBehavior) => { + ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) + ;(readFile as jest.Mock).mockResolvedValue(input) + + await convertToMDX('test.md') + + const converted = (writeFile as jest.Mock).mock.calls[0][1] + assertCurrentBehavior(converted) +}) + +it('removes multi-line compound absolute imports', async () => { + const importBlock = `import ChatbotHeader, { + ChatbotHeaderMain, + ChatbotHeaderActions, +} from '@patternfly/chatbot/dist/dynamic/ChatbotHeader'` + ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) + ;(readFile as jest.Mock).mockResolvedValue(`${importBlock}\n# Heading`) + + await convertToMDX('test.md') + + const converted = (writeFile as jest.Mock).mock.calls[0][1] + expect(converted).not.toContain('import ChatbotHeader') + expect(converted).not.toContain('ChatbotHeaderMain') + expect(converted).toContain('# Heading') +}) + +it('removes combined imports before generating colliding example imports', async () => { + const mockContent = `import Chatbot, { ChatbotDisplayMode } from '@patternfly/chatbot/dist/dynamic/Chatbot' +\`\`\`tsx file="./Chatbot.tsx" isFullscreen +\`\`\` +\`\`\`tsx file="./ChatbotDisplayMode.tsx" +\`\`\`` + ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) + ;(readFile as jest.Mock).mockResolvedValue(mockContent) + + await convertToMDX('test.md') + + const converted = (writeFile as jest.Mock).mock.calls[0][1] + expect(converted).not.toContain("import Chatbot, { ChatbotDisplayMode } from '@patternfly/chatbot/dist/dynamic/Chatbot'") + expect(converted).toContain('import Chatbot from "./Chatbot.tsx?raw"') + expect(converted).toContain('') + expect(converted).toContain('import ChatbotDisplayMode from "./ChatbotDisplayMode.tsx?raw"') + expect(converted).toContain('') +}) + +it('removes type-only absolute imports so MDX never sees TypeScript-only syntax', async () => { + const mockContent = `import type Foo from '@patternfly/some-package' +import type { Bar } from '@patternfly/some-package' +import Kept from './Kept' +\`\`\`tsx file="./Foo.tsx" +\`\`\`` + ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) + ;(readFile as jest.Mock).mockResolvedValue(mockContent) + + await convertToMDX('test.md') + + const converted = (writeFile as jest.Mock).mock.calls[0][1] + expect(converted).not.toContain("import type Foo from '@patternfly/some-package'") + expect(converted).not.toContain("import type { Bar } from '@patternfly/some-package'") + expect(converted).toContain("import Kept from './Kept'") + expect(converted).toContain('import Foo from "./Foo.tsx?raw"') + expect(converted).toContain('') +}) + it('should convert a file with HTML examples', async () => { const mockContent = '# Test Content\n```html\n
Test HTML
\n```' const expectedContent = diff --git a/cli/convertToMDX.ts b/cli/convertToMDX.ts index 5fcae3a..c7c6558 100644 --- a/cli/convertToMDX.ts +++ b/cli/convertToMDX.ts @@ -76,10 +76,28 @@ function removeNoLiveTags(content: string): string { return content.replace(/```no[lL]ive/g, '```') } +// Binding shapes between `import` and `from` that should be removed. +// Ordered longest-first so the compound form wins before its default-only prefix. +const removableBindingForms = [ + String.raw`[\w$]+\s*,\s*\{[\w$\s,]*\}`, // import Chatbot, { ChatbotDisplayMode } + String.raw`[\w$]+`, // import Chatbot + String.raw`\{[\w$\s,]*\}`, // import { Foo, Bar as Baz } +] + +// Module path that is absolute (not ./ or ../) and not a CSS file +const absoluteNonCssModule = String.raw`['"](?!\.\.?\/)(?!.*\.css['"])[^'"]*['"]` + +// Optional TypeScript-only `type` keyword (e.g. import type Foo from 'pkg') +const optionalTypeKeyword = String.raw`(?:type\s+)?` + +const removableImportRegex = new RegExp( + String.raw`^import ${optionalTypeKeyword}(?:${removableBindingForms.join('|')}) from ${absoluteNonCssModule};?\n`, + 'gm', +) + function removeExistingImports(content: string): string { // Remove imports that are absolute and not CSS - const importRegex = /^import {?[\w\s,\n]*}? from ['"](?!\.\.?\/)(?!.*\.css['"])[^'"]*['"];?\n/gm - return content.replace(importRegex, '') + return content.replace(removableImportRegex, '') } function convertCommentsToMDX(content: string): string { From de3718cde21ff58c6ade4ee18fd06bbd473b357c Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Mon, 26 Oct 2026 13:18:42 -0400 Subject: [PATCH 2/2] fix: review update --- cli/__tests__/convertToMDX.test.ts | 33 ++++++++++++++++++++++++------ cli/convertToMDX.ts | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cli/__tests__/convertToMDX.test.ts b/cli/__tests__/convertToMDX.test.ts index 0f2777a..907997b 100644 --- a/cli/__tests__/convertToMDX.test.ts +++ b/cli/__tests__/convertToMDX.test.ts @@ -172,6 +172,22 @@ it.each<[string, string, (converted: string) => void]>([ "import * as React from 'react'\n# Heading", (converted) => expect(converted).toContain("import * as React from 'react'"), ], + [ + 'bound absolute CSS imports with a ?url suffix are stripped', + "import styles from '@patternfly/react-core/dist/styles/base.css?url'\n
", + (converted) => { + expect(converted).not.toContain("import styles from '@patternfly/react-core/dist/styles/base.css?url'") + expect(converted).toContain('
') + }, + ], + [ + 'bound absolute CSS imports with a ?inline suffix are stripped', + "import styles from '@patternfly/react-core/dist/styles/base.css?inline'\n
", + (converted) => { + expect(converted).not.toContain("import styles from '@patternfly/react-core/dist/styles/base.css?inline'") + expect(converted).toContain('
') + }, + ], ])('known limitation (documents current behavior; changing it is a deliberate decision): %s', async (_desc, input, assertCurrentBehavior) => { ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) ;(readFile as jest.Mock).mockResolvedValue(input) @@ -198,12 +214,17 @@ it('removes multi-line compound absolute imports', async () => { expect(converted).toContain('# Heading') }) -it('removes combined imports before generating colliding example imports', async () => { - const mockContent = `import Chatbot, { ChatbotDisplayMode } from '@patternfly/chatbot/dist/dynamic/Chatbot' -\`\`\`tsx file="./Chatbot.tsx" isFullscreen -\`\`\` -\`\`\`tsx file="./ChatbotDisplayMode.tsx" -\`\`\`` +it.each([ + ['LF', '\n'], + ['CRLF', '\r\n'], +])('removes combined imports before generating colliding example imports with %s line endings', async (_lineEndingName, lineEnding) => { + const mockContent = [ + "import Chatbot, { ChatbotDisplayMode } from '@patternfly/chatbot/dist/dynamic/Chatbot'", + '\`\`\`tsx file="./Chatbot.tsx" isFullscreen', + '\`\`\`', + '\`\`\`tsx file="./ChatbotDisplayMode.tsx"', + '\`\`\`', + ].join(lineEnding) ;(glob as unknown as jest.Mock).mockResolvedValue(['test.md']) ;(readFile as jest.Mock).mockResolvedValue(mockContent) diff --git a/cli/convertToMDX.ts b/cli/convertToMDX.ts index c7c6558..f831450 100644 --- a/cli/convertToMDX.ts +++ b/cli/convertToMDX.ts @@ -91,7 +91,7 @@ const absoluteNonCssModule = String.raw`['"](?!\.\.?\/)(?!.*\.css['"])[^'"]*['"] const optionalTypeKeyword = String.raw`(?:type\s+)?` const removableImportRegex = new RegExp( - String.raw`^import ${optionalTypeKeyword}(?:${removableBindingForms.join('|')}) from ${absoluteNonCssModule};?\n`, + String.raw`^import ${optionalTypeKeyword}(?:${removableBindingForms.join('|')}) from ${absoluteNonCssModule};?\r?\n`, 'gm', )