Skip to content
This repository was archived by the owner on Apr 27, 2026. It is now read-only.

Fix/starlight components auto allow imports - #152

Merged
jp-knj merged 12 commits into
mainfrom
fix/starlight-components-auto-allow-imports
Jan 25, 2026
Merged

jp-knj merged 12 commits into
mainfrom
fix/starlight-components-auto-allow-imports

Conversation

@jp-knj

@jp-knj jp-knj commented Jan 25, 2026

Copy link
Copy Markdown
Member

Summary

Linked Issues

  • Closes #

Tasks from TODO.md

  • Task 1.x: ...

Performance Evidence

Verification Steps

  1. npm install && npm run build:napi
  2. node scripts/smoke-napi.mjs samples/large.md
  3. Result: ...

Checklist

  • cargo fmt executed?
  • cargo clippy passed?
  • Tests passed?
  • No secrets included?

jp-knj and others added 11 commits January 25, 2026 01:17
…ules

- Replace duplicate type definitions with imports from vite-plugin/types.ts
- Replace duplicate function definitions with imports from extracted modules
  (binding-loader, jsx-module, directive-rewriter)
- Replace duplicate path utilities with imports from utils/paths.ts
- Add normalize-config.ts with normalizeStarlightComponents helper
- Add shiki-highlighter.ts with createShikiHighlighter function
- Update vite-plugin/index.ts to export new modules

Reduces vite-plugin.ts from 1267 to 673 lines (~47% reduction).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Import stripHeadingsMeta from utils/validation.ts instead of defining
it locally in transforms/inject-components.ts.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove duplicate DEFAULT_EXTENSIONS from vite-plugin/types.ts (use utils/paths.ts)
- Remove duplicate ShikiHighlighter type from vite-plugin/shiki-highlighter.ts (use transforms/shiki.ts)
- Remove duplicate parse5 types from transforms/shiki.ts (use vite-plugin/types.ts)
- Use sanitizeHtmlForJsx function instead of inline brace escaping in blocks-to-jsx.ts

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove deprecated constants from config.ts and inject-components.ts:
- STARLIGHT_COMPONENTS, STARLIGHT_COMPONENTS_MODULE
- EXPRESSIVE_CODE_COMPONENT, EXPRESSIVE_CODE_MODULE
- ASTRO_COMPONENTS, ASTRO_COMPONENTS_MODULE

These were deprecated in favor of using registry libraries directly from
markflow/registry. Tests now derive expected values from the libraries.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Conflict resolution accidentally removed required imports for esbuild
and rollup types in vite-plugin.ts.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 1 - Measurement Infrastructure:
- Add debug timing logs (MARKFLOW_DEBUG_TIMING=1) in vite-plugin.ts
- Add Criterion benchmarks for to_blocks, smartypants, slugify, jsx_normalize

Phase 2 - Quick Wins:
- Add Shiki early skip when no <pre> tags exist (avoids parse5 overhead)
- Optimize regex in inject-components.ts: O(n) -> O(1) compilation
- Add code block early detection to skip shiki for files without code blocks

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… is enabled

When using `starlightComponents: true` directly (without a preset),
the default Starlight allow import patterns are now automatically
applied. This ensures imports like `@astrojs/starlight/components`
don't trigger fallback to @mdx-js/mdx.

Previously, only using `starlightPreset()` would set up allowImports,
but the shorthand `starlightComponents: true` option left mdx undefined,
causing all imports to be treated as disallowed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add hoisted_exports support in Rust codegen for user-defined exports
- Enhance MDX pattern detection to return detailed results (exports, imports)
- Add fallback statistics logging (exports count, disallowed imports count)
- Log top disallowed import sources when many files fallback
- Add STARLIGHT_DEFAULT_ALLOW_IMPORTS constant with common patterns
- Support allowImports option in starlightPreset for custom patterns

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

Code review

I found one issue during the review:

Unused function and unpopulated field

Files affected:

  • packages/astro-markflow/src/utils/mdx-detection.ts (lines 72-80)
  • packages/astro-markflow/src/vite-plugin.ts (lines 300-301)

Issue:
The extractExportStatements function is defined but never called anywhere in the codebase. Additionally, the MdxPatternDetectionResult.exports field is never populated by detectProblematicMdxPatterns.

This means the statistics tracking code in vite-plugin.ts that checks detection.exports will always find it undefined, resulting in fallbackStats.exports always being 0.

Impact:

  • The fallback statistics breakdown will never show "X with exports"
  • The extractExportStatements function is dead code
  • The statistics are misleading (will always show 0 for exports)

Suggested fix:
Either:

  1. Remove the unused extractExportStatements function and the exports field from the interface if exports are now fully handled by Rust and don't need tracking, OR
  2. Populate detection.exports by calling extractExportStatements in detectProblematicMdxPatterns if export tracking is still desired

Code references:

  • Unused function:
    */
    function extractExportStatements(content: string): string[] {
    const exports: string[] = [];
    const exportRegex = /^export\s+(const|let|var|function|class|default)\b/gm;
    let match;
    while ((match = exportRegex.exec(content)) !== null) {
    exports.push(`export ${match[1]}`);
    }
    return exports;
    }
  • Unpopulated field checked here:
    // Track statistics
    if (detection.exports && detection.exports.length > 0) {
    fallbackStats.exports++;
    } else if (detection.disallowedImports && detection.disallowedImports.length > 0) {
  • Field definition but never populated:
    return {
    hasProblematicPatterns: true,
    reason: `Contains imports with no allowImports configured: ${allImports.slice(0, 3).join(', ')}${allImports.length > 3 ? ` (+${allImports.length - 3} more)` : ''}`,
    allImports,
    disallowedImports: allImports,
    };
    }
    // Check if all imports are from allowed sources
    const disallowedImports = allImports.filter(
    (src) => !isAllowedImport(src, allowImports)
    );
    if (disallowedImports.length > 0) {
    return {
    hasProblematicPatterns: true,
    reason: `Contains disallowed imports: ${disallowedImports.slice(0, 3).join(', ')}${disallowedImports.length > 3 ? ` (+${disallowedImports.length - 3} more)` : ''}`,
    allImports,
    disallowedImports,
    };
    }
    return {
    hasProblematicPatterns: false,
    allImports,
    };
    }

Summary: 1 issue found. Checked for bugs and CLAUDE.md compliance.

… functions

- Remove unused MdxPatternDetectionResult type import from vite-plugin
- Remove deprecated transformInjectAstroComponents and
  transformInjectStarlightComponents (replaced by transformInjectComponentsFromRegistry)
- Remove corresponding tests for deprecated functions
- Remove unused exports tracking from mdx-detection (exports are now
  handled by Rust via hoisting)
- Remove extractExportStatements function and exports field from interface

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jp-knj
jp-knj merged commit e92e5e5 into main Jan 25, 2026
17 of 18 checks passed
@jp-knj
jp-knj deleted the fix/starlight-components-auto-allow-imports branch January 25, 2026 08:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant