Skip to content

Conversation

@wise-king-sullyman
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman commented Dec 15, 2025

Closes #188

Summary by CodeRabbit

  • Bug Fixes
    • Improved import removal logic during MDX conversion to correctly handle absolute imports that aren't CSS files, enhancing the content transformation process.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

Walkthrough

The import removal logic in the MDX conversion CLI has been refined to preserve local relative imports while still removing absolute imports (excluding CSS). This change allows local JavaScript and JSON imports in markdown files to persist during conversion, enabling example files to retain necessary data dependencies.

Changes

Cohort / File(s) Change Summary
Import Filtering Logic
cli/convertToMDX.ts
Updated import removal condition to distinguish between relative imports (starting with ./ or ../) and absolute imports. Now preserves local relative imports while removing absolute imports that don't end with .css, broadening the scope of stripped imports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file modification with localized logic change
  • The conditional update is straightforward but requires verification that the new path checking logic (not starting with ./ or ../) correctly identifies absolute vs. relative imports across all use cases
  • Consider testing with the example files mentioned in the linked issue (galleryData, tokensTable) to ensure local imports are preserved as intended

Poem

🐰 A simple tweak to the import gate,
Now local paths won't meet their fate,
Relative imports skip the shears,
While absolute ones fade away, my dears,
Data stays, examples cheer! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the logic to preserve relative imports during md->mdx conversion, which aligns with the code change.
Linked Issues check ✅ Passed The PR implementation matches the requirement from issue #188: non-relative imports are now removed while relative imports (starting with ./ or ../) are preserved.
Out of Scope Changes check ✅ Passed The changes are entirely focused on the import removal logic in the MDX conversion tool, directly addressing the stated objective with no extraneous modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch avoid-removing-relative-imports

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14af6cb and 2b29db2.

📒 Files selected for processing (1)
  • cli/convertToMDX.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build (22.x)
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
cli/convertToMDX.ts (1)

49-50: The regex pattern correctly handles the intended use case.

The updated regex on lines 49-50 works as designed: it removes absolute imports while preserving relative imports (starting with ./ or ../) and CSS imports. The test suite at cli/__tests__/convertToMDX.test.ts (lines 113-135) validates this behavior.

The edge cases mentioned in the original review are not concerns for this codebase:

  • Side-effect imports like import './styles.css' are correctly preserved because they lack the from keyword that the regex requires
  • Mixed imports, type imports, and namespace imports do not appear in the markdown files
  • No imports without trailing newlines were found

The current implementation is sufficient and solves the stated problem without needing refactoring.

Likely an incorrect or invalid review comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

Deploying patternfly-doc-core with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2b29db2
Status: ✅  Deploy successful!
Preview URL: https://e2f15afd.patternfly-doc-core.pages.dev
Branch Preview URL: https://avoid-removing-relative-impo.patternfly-doc-core.pages.dev

View logs

@dlabaj dlabaj merged commit 11156f9 into main Dec 17, 2025
5 checks passed
@github-actions
Copy link

🎉 This PR is included in version 1.15.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only remove non-local imports from markdown files during md -> mdx conversion

3 participants