feat(mdx): add support for MDX file handling and compilation#27047
Open
benpsnyder wants to merge 37 commits intooven-sh:mainfrom
Open
feat(mdx): add support for MDX file handling and compilation#27047benpsnyder wants to merge 37 commits intooven-sh:mainfrom
benpsnyder wants to merge 37 commits intooven-sh:mainfrom
Conversation
- Introduced new MDX loader and related APIs to handle MDX files. - Updated existing loaders to include MDX support in various components, including the bundler, transpiler, and module loader. - Added MDX compilation functionality, allowing MDX source to be processed into JSX. - Enhanced error handling for MDX compilation failures. - Updated documentation and type definitions to reflect new MDX capabilities. This commit expands the functionality of the Bun framework to support MDX, enhancing its capabilities for handling markdown with embedded JSX components.
- Added support for `.mdx` file handling, including new loaders and compilation capabilities. - Updated the `isJavaScriptLike` function to recognize `.mdx` files. - Introduced new test cases for MDX compilation, including handling of typed arrays, option aliases, and error scenarios. - Added multiple fixture MDX files to validate various use cases and ensure robust testing of the MDX loader and compiler. This commit significantly improves the MDX functionality within the Bun framework, allowing for more versatile markdown processing with embedded JSX components.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds first-class MDX support: new mdx loader enum and mappings, public Bun.mdx.compile API and Bun.mdx runtime binding, Zig MDX compiler and JSX renderer, transpiler/parser/bundler/print integrations, CLI/dev-server MDX mode, C++ binding updates, tests, and fixtures. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/run_command.zig (1)
1536-1547: 🧹 Nitpick | 🔵 TrivialDuplicated glob-entry logic for HTML and MDX — consider consolidating.
The
.htmland.mdxblocks are structurally identical. A minor refactor would reduce duplication:♻️ Optional: consolidate the two glob checks
} else |_| { - // Support globs for HTML entry points. - if (strings.hasSuffixComptime(target_name, ".html")) { - if (strings.containsChar(target_name, '*')) { - return _bootAndHandleError(ctx, target_name, .html); - } - } - if (strings.hasSuffixComptime(target_name, ".mdx")) { - if (strings.containsChar(target_name, '*')) { - return _bootAndHandleError(ctx, target_name, .html); - } - } + // Support globs for HTML and MDX entry points. + if (strings.hasSuffixComptime(target_name, ".html") or strings.hasSuffixComptime(target_name, ".mdx")) { + if (strings.containsChar(target_name, '*')) { + return _bootAndHandleError(ctx, target_name, .html); + } + } }
🤖 Fix all issues with AI agents
In `@src/bun.js/api/MdxObject.zig`:
- Around line 2-6: Replace usages of ZigString with bun.String: change
ZigString.static("compile") to the bun.String equivalent used elsewhere (e.g.,
bun.String.createUTF8ForJS or bun.String.static-like API), replace
ZigString.init("") and any .toZigString() conversions with the bun.String
creation/conversion helpers already used in this file (matching the pattern at
the existing bun.String.createUTF8ForJS call), and then remove the ZigString
alias declaration (const ZigString = jsc.ZigString;) near the bottom; update
references to the ZigString symbols to use bun.String functions (e.g., for the
compile key and empty string init) so bridging is consistent with the other
bun.String uses.
- Around line 54-57: Replace the current optional string check that calls
opts_value.get and then tests import_source_value.isString() for the
jsxImportSource option with the project-standard helper getStringish so
undefined/null are treated as absent; specifically, in the code handling
"jsxImportSource" use opts_value.getStringish(globalThis, "jsxImportSource") (or
the equivalent getStringish helper already used elsewhere) and then only call
globalThis.throwInvalidArguments("jsxImportSource must be a string", .{}) if
getStringish returns an explicit non-string error, ensuring the behavior matches
ServerConfig.zig's handling of optional strings like "hostname"/"unix".
In `@src/bun.js/bindings/BunObject`+exports.h:
- Around line 14-16: The getter export list is out of alphabetical order: move
the macro(mdx) entry so it comes after macro(markdown) (i.e., reorder the two
lines where macro(mdx) and macro(markdown) appear) to keep the lowercase entries
alphabetized; ensure the rest of the macro(...) entries remain unchanged.
In `@src/bun.js/bindings/headers-handwritten.h`:
- Around line 258-260: The enum values for MD/MDX are inconsistent between
headers; update the definitions in src/bun.js/bindings/headers-handwritten.h so
BunLoaderTypeMD and BunLoaderTypeMDX match the plugin API (BUN_LOADER_MD = 20,
BUN_LOADER_MDX = 21) by changing BunLoaderTypeMD to 20 and BunLoaderTypeMDX to
21 (ensure BunLoaderTypeYAML remains correct), then rebuild and run
tests/compile a native plugin to verify the values are aligned with
bundler_plugin.h's BUN_LOADER_MD and BUN_LOADER_MDX.
In `@src/js/internal/html.ts`:
- Around line 10-26: The top-level require("internal/mdx") causes MDX code to
load even when not needed; remove the module-level require and instead lazy-load
inside start(): when shouldUseMdxMode(argv) is true, require or dynamic import
"internal/mdx" (e.g., const mdxInternal = require("internal/mdx") or await
import("internal/mdx")) and then call mdxInternal(); keep shouldUseMdxMode and
start as-is and ensure argv is available in start before invoking
shouldUseMdxMode.
In `@src/js/internal/mdx.ts`:
- Line 399: The module currently does a bare export ("export default start;")
which violates the internal-module convention; replace the bare value export by
exporting an object default containing the symbol(s) from this module (e.g.,
include the start symbol) so the module uses "export default { start }" style;
update any callers that expect the bare export to access the property on the
default object (e.g., caller.start) if necessary.
- Around line 76-91: The hostname/port parsing in the arg-handling block
(variables hostname, port) incorrectly uses hostname.split(":") which breaks for
IPv6; update the logic to detect bracketed IPv6 (hostname startsWith("[") and
contains "]") and extract the host between brackets and any trailing :port,
otherwise split using hostname.lastIndexOf(":") to separate host and port so
IPv6 addresses with multiple colons are preserved; apply this fix in both places
where hostname is parsed (the sections handling "--hostname=" and "--host=") and
ensure port is parsed with parseInt only when a port substring is present.
- Around line 65-164: Refactor start() to separate flag parsing from file
collection: first pass over argv should only handle options and flags (set
hostname, port, enableConsoleLog, handle --help) using the existing option
checks on argv entries and skip any entries that endWith(".mdx") or are
file-like; second pass should collect and resolve MDX files (handle globs with
Bun.Glob.scanSync, resolve with Bun.resolveSync, skip node_modules paths, and
dedupe into args) using the existing glob/file-resolution logic; keep the same
variables (argv, args, hostname, port, enableConsoleLog) and helper calls
(Bun.Glob, Bun.resolveSync) but move them into the dedicated file-collection
loop so the flow is clearer and the current continue branches are removed.
- Around line 233-241: The temp directory is created in the current working
directory via tmpRoot (path.join(cwd, `.bun-mdx-${process.pid}`)) and may be
orphaned if the process is killed; change tmpRoot to use the OS temp directory
(os.tmpdir()) instead (e.g., path.join(os.tmpdir(),
`.bun-mdx-${process.pid}-${uniqueId}`)) and keep calling ensureDir(tmpRoot);
retain the process.on("exit", ...) cleanup but note SIGKILL cannot be
handled—using os.tmpdir() avoids polluting the project root and reduces orphaned
files.
- Around line 35-63: The HTML template in emitMdxHtmlShell interpolates the
unescaped title (derived from path.basename(mdxPath)) directly into the <title>
tag, enabling HTML injection; fix by HTML-escaping the title before
interpolation (escape at least &, <, and > and optionally " and ')—either add a
small helper like escapeHtml and call it when building the template in
emitMdxHtmlShell, or ensure the caller passes an already-escaped title; update
references to title in emitMdxHtmlShell to use the escaped value.
- Around line 161-163: The Set-based deduplication of the args array is being
run inside the loop (args = [...new Set(args)]), causing repeated array
reconstruction; remove that line from the loop and instead, after the loop
finishes, perform deduplication once (e.g., if (args.length > 1) args = [...new
Set(args)]). Locate and update the code that manipulates the args variable in
src/js/internal/mdx.ts so deduplication happens only once after the loop exits.
In `@src/md/jsx_renderer.zig`:
- Around line 114-125: The writeAttrEscaped function in JSXRenderer currently
converts '{' and '}' into "{'{'}" and "{'}'}", corrupting quoted attribute
values; update writeAttrEscaped to stop treating braces as JSX escapes by
removing the '{' and '}' switch arms (or, if you prefer explicit HTML entities,
replace those arms to write "{" and "}" instead), ensuring all other
cases remain unchanged and that calls still use self.write/self.writeChar as
before.
In `@src/md/mdx.zig`:
- Around line 247-282: The frontmatter handling in emitFrontmatterAsJson
currently tokenizes lines and treats every value as a simple string; replace
this with the project YAML parser to correctly handle arrays, booleans, numbers
and multiline values: parse yaml_content into a structured value
(map/array/scalar) using the existing YAML parsing utility, then walk that
structure and serialize it to JSON using the same output buffer and
appendJsonStringEscaped for string escaping; update emitFrontmatterAsJson to
call the parser, handle parse errors (bubble up the error), and remove the
manual line-splitting fallback (or, if you intentionally restrict features, add
a clear comment/docstring on emitFrontmatterAsJson describing the supported YAML
subset instead).
- Around line 50-90: extractTopLevelStatements currently splits source by
newline (lines via std.mem.splitScalar) and only treats a single line as an
import/export (maybe_stmt), which breaks multi-line ESM statements; change the
logic so when you detect the start of a top-level ESM statement (use maybe_stmt
and in_code_fence false) you accumulate subsequent lines from the iterator into
a buffer until the statement terminator is reached (e.g., a trailing semicolon,
a closing brace followed by optional whitespace and "from", or EOF), then append
that full accumulated text to stmts (instead of only the single line); reference
symbols: extractTopLevelStatements, lines, maybe_stmt, in_code_fence, stmts,
remaining.
- Around line 92-168: In replaceExpressions, the brace depth logic
(expr_start/depth) currently counts every '{' and '}', which breaks when braces
appear inside JS strings, template literals, or comments; update the scanner
inside the expr_start != null branch to skip over string literals (handling
escapes for ' and "), template literals (handle backticks and nested ${...}
expressions which must re-enter brace counting), and both // and /* */ comments
so only structural braces affect depth; preserve existing behavior for nested
braces and continue to build slot placeholders and output as before (refer to
expr_start, depth, and the loop that appends to slots/output).
In `@src/transpiler.zig`:
- Around line 1403-1489: Summary: The MDX handler duplicates ~30 parser option
assignments already present in the JS/TS handler; extract them into a shared
helper to avoid copy-paste. Fix: add a helper method on Transpiler (suggested
name buildJsParserOptions) that takes the common inputs (transpiler, jsx/js
parse config object, this_parse, file_hash and any allocator/context needed) and
returns a fully populated js_parser.Parser.Options; replace the duplicated block
in the .mdx branch and the .js/.jsx/.ts/.tsx branch by calling
Transpiler.buildJsParserOptions(...) and then apply only per-branch overrides
(e.g., in the .mdx handler set jsx.parse = true and any different
trim_unused_imports/defaults, set opts.macro_context and opts.features.*
per-target as currently done). Ensure the helper initializes via
js_parser.Parser.Options.init(...) and preserves setting opts.macro_context,
opts.repl_mode, opts.filepath_hash_for_hmr etc., so callers only mutate the few
branch-specific fields (variables referenced: transpiler, this_parse, opts,
file_hash, js_parser.Parser.Options.init).
- Around line 1403-1413: The catch currently swallows the error from
bun.md.mdx.compile and logs a generic message; change the failure branch to
capture the thrown error (e.g., catch |err|) and pass its message/diagnostics
into transpiler.log.addErrorFmt so the logged text includes the error details
(for example "Failed to compile MDX: {err}" or include
err.message/err.toString/diagnostic fields if present) while still returning
null; update the .mdx arm around bun.md.mdx.compile and the
transpiler.log.addErrorFmt call (logger.Loc.Empty, transpiler.allocator) to
include that captured error information.
- Around line 1481-1487: The MDX branch currently maps every .already_bundled
case to .source_code, dropping CJS/bytecode variants; update the MDX handling
around the .already_bundled match (the block that sets .ast, .already_bundled,
.source, .loader, .input_fd) to either (A) mirror the JS/TS path and inspect the
actual bundle variant returned by bun.md.mdx.compile(), preserving .bun_cjs and
.bytecode_cjs into .already_bundled when present, or (B) add a short comment
next to this mapping explicitly stating that bun.md.mdx.compile() always yields
ESM so collapsing to .source_code is intentional; reference the already_bundled
enum and bun.md.mdx.compile() when making the change so behavior is explicit and
consistent.
In `@test/js/bun/md/mdx.test.ts`:
- Around line 213-349: Tests parse stdout with hardcoded regexes and box-drawing
characters; extract reusable named constants (e.g., URL_REGEX and BOX_TREE_REGEX
or ROUTE_ENTRY_REGEX) at the top of the test file and replace inline regex
literals (/url:\s*(\S+)/ and /\u2514\u2500\u2500/) and the route-entry matches
with those constants to avoid duplication and make future changes easier; update
places that call readUntil and output.match (used in the "MDX direct serve
mode", "route matrix index/docs/guides", and "overlapping glob dedupe check"
tests) to use URL_REGEX and use BOX_TREE_REGEX/ROUTE_ENTRY_REGEX for detecting
the tree output, and consider replacing the terse for...of assertions in the
"route matrix index/docs/guides" test with explicit per-route checks for clearer
failures.
- Around line 182-204: The readUntil helper (function readUntil, reading from
Bun.Subprocess.stdout via reader.read and predicate) can hang if stdout remains
open and no matching output appears; update readUntil to race the reader loop
against a timeout promise (configurable or fixed) so that if the timeout elapses
you release the reader lock and throw or return a descriptive timeout
error/value; ensure the Promise.race wraps the await reader.read loop, always
calls reader.releaseLock() in finally, and documents/exports the timeout so
tests can adjust it as needed.
- Around line 206-211: The tests currently maintain a redundant global running:
Bun.Subprocess[] and afterEach cleanup while each serve-mode test already uses
the RAII-style await using proc = Bun.spawn(...), so remove the running array
and the afterEach block (and any pushes to running) to avoid duplicate
subprocess teardown; ensure each serve test continues to use await using proc
(or, if you prefer centralized cleanup instead, remove await using usages and
push procs into running from spawn calls) and verify no references to running
remain in mdx.test.ts.
- Updated the logic in `processInlineContent` to allow for hard breaks when the `hard_soft_breaks` flag is set, enhancing the flexibility of break handling. - Added new tests to validate the behavior of hard and soft breaks, ensuring consistent output across different configurations. - Refactored test cases to improve clarity and maintainability, including regex updates for route entries and URL matching. This commit enhances the MDX processing capabilities, ensuring better compliance with user-defined break preferences.
Contributor
Code ReviewNewest first 🔴 af593 — 2 issue(s) found
Powered by Claude Code Review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Updated the `create` function in `MdxObject.zig` to use `bun.String` for the "compile" method. - Enhanced the `parseOptions` function to simplify string handling for the `jsxImportSource` option, improving memory management and readability. - Adjusted BunLoaderType constants in `headers-handwritten.h` to correct loader type values for YAML, MD, and MDX. - Cleaned up test code in `mdx.test.ts` by removing unnecessary subprocess management, enhancing test clarity. These changes improve the overall efficiency and maintainability of the MDX handling within the Bun framework.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/bindings/headers-handwritten.h (1)
244-260:⚠️ Potential issue | 🔴 CriticalEnum values in
headers-handwritten.hare not in sync withschema.zig.Line 244 mandates sync with
bun.schema.api.Loaderinschema.zig, but the actual values diverge:
BunLoaderTypeYAML: 18 (schema.zig: 19)BunLoaderTypeMD: 20 (schema.zig: 21)BunLoaderTypeMDX: 21 (schema.zig: 22)All three values are consistently off by 1. Update to match schema.zig's enum definition.
🤖 Fix all issues with AI agents
In `@src/bun.js/api/MdxObject.zig`:
- Around line 65-88: The helper camelCaseOf currently treats a leading
underscore as a signal to capitalize the first real character (turning
"_private" into "Private"); update camelCaseOf so that the capitalization branch
only applies to non-leading characters (e.g., require i != 0 before applying
cap_next) while keeping the existing logic that strips underscores and ignores
trailing underscores; locate the logic in the for loop inside camelCaseOf that
sets buf[i] and change the capitalization condition to also check that this is
not the first output character so leading underscores do not cause PascalCase.
- Around line 30-33: The current catch on mdx.compile maps every non-OutOfMemory
error to a generic SyntaxError via globalThis.createSyntaxErrorInstance which
can misclassify future errors; update the catch to inspect the err value instead
of the catch-all else — for known MDX-specific parsing errors keep creating a
SyntaxError via globalThis.createSyntaxErrorInstance, but for other error types
(e.g., IoError, InvalidUtf8, etc.) either rethrow the original error using
globalThis.throwValue with a more appropriate error instance or include the
actual error name/details in the thrown message so mdx.compile, the
globalThis.throwOutOfMemory, globalThis.throwValue, and
globalThis.createSyntaxErrorInstance usages correctly preserve error semantics
and diagnostic information.
- Around line 54-59: The code currently accepts non-string values for
jsxImportSource because opts_value.getStringish() coerces them; before using the
coerced string, add a type guard that checks opts_value.get(globalThis,
"jsxImportSource").?.isString() and call
globalThis.throwInvalidArguments("jsxImportSource must be a string", .{}) if
it's not a string; keep the existing defer str.deref(), utf8 conversion and
assign to options.jsx_import_source via allocator.dupe(u8, utf8.slice()) only
when the check passes (or alternatively modify getStringish() to perform this
string-only validation).
In `@test/js/bun/md/mdx.test.ts`:
- Around line 115-120: The Bun.spawn calls that create proc (the variable named
proc in the tests invoking Bun.spawn for "entry.tsx") omit the explicit stdout
option yet later call proc.stdout.text(); update each Bun.spawn invocation (the
ones at the blocks creating proc for the loader integration tests) to include
stdout: "pipe" alongside stderr: "pipe" so proc.stdout is explicitly piped and
the reads (proc.stdout.text()) are consistent and readable.
Contributor
Code ReviewNewest first 🔴 8200a — 1 issue found
Powered by Claude Code Review |
- Added handling for `JSError`, `JSTerminated`, and `StackOverflow` errors in the `compile` function of `MdxObject.zig`, improving robustness during compilation. - Refined the `parseOptions` function to ensure `jsxImportSource` is validated as a string and only processed if not undefined or null, enhancing error handling and memory management. - Introduced file watching for `.mdx` source files in `mdx.ts` to enable automatic recompilation on changes, improving development workflow. - Cleaned up test cases in `mdx.test.ts` by ensuring consistent output handling, enhancing test reliability. These changes improve the overall stability and usability of the MDX processing within the Bun framework.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/js/internal/mdx.ts`:
- Around line 304-331: The retry loop reuses the same failing port because
defaultPort is initialized to the original port value; change how defaultPort is
derived so the first retry uses the next port. Compute a basePort from port ||
parseInt(env.PORT || env.BUN_PORT || env.NODE_PORT || "3000", 10) and set
defaultPort = basePort + 1 (or pre-increment before use), then call Bun.serve
with that incremented port in the loop (references: variable defaultPort,
basePort computation, Bun.serve inside the retry try/catch) so the first retry
does not attempt the same occupied port.
In `@src/md/mdx.zig`:
- Around line 271-282: The function appendJsonStringEscaped only handles a
subset of control chars; update appendJsonStringEscaped to escape every Unicode
codepoint U+0000..U+001F per JSON rules by handling known single-char escapes
(backspace, formfeed, newline, carriage return, tab, backslash, quote) and
emitting a \u00XX hex escape for any other control byte value; modify the switch
over c in appendJsonStringEscaped to map '\b' and '\f' to "\\b" and "\\f" and to
call a helper or inline logic that formats unknown control bytes as four-digit
hex "\u00" + two hex digits (using the allocator and out.appendSlice/append as
appropriate) so all control characters produce valid JSON output.
In `@test/js/bun/md/mdx.test.ts`:
- Around line 98-182: In the "malformed mdx import reports compile failure"
test, the Bun.spawn call that creates proc does not set stdout and leaves the
pipe unused; update the Bun.spawn options for the proc in that test (the proc
variable inside the test "malformed mdx import reports compile failure") to
include stdout: "ignore" (or stdout: "pipe" and consume proc.stdout) so the
stdout pipe cannot fill and block the test runner.
- Introduced a symlink for the `node_modules` directory in multiple MDX test cases to ensure consistent module resolution during tests. - This change enhances the reliability of the tests by providing access to necessary dependencies, improving the overall test environment setup. These updates contribute to a more stable testing framework for MDX within the Bun ecosystem.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@test/js/bun/md/mdx.test.ts`:
- Line 116: Extract the repeated fs.symlinkSync(repoNodeModules,
path.join(String(dir), "node_modules"), "junction") into a small helper (e.g.,
linkNodeModules(dir: string)) and replace the seven inline calls with calls to
this helper; update usages that pass String(dir) to simply pass dir and ensure
the helper wraps repoNodeModules, path.join(dir, "node_modules"), and the
"junction" flag so future changes to linking strategy only need to be made in
linkNodeModules.
- Around line 79-97: The test currently verifies each .mdx file has expectations
but doesn't fail if expectByFile contains stale keys; after building files (the
const files = ... and before/after the loop over files) add an assertion that
every key in expectByFile has a matching file in files (e.g. compute
Object.keys(expectByFile).filter(k => !files.includes(k)) and expect that array
to be empty) so stale entries in expectByFile cause the test to fail; reference
expectByFile, files, and fixtureDir when locating where to add the check.
- Around line 197-219: The readUntil helper currently returns accumulated stdout
when the stream ends without the predicate matching, causing downstream
non-descriptive TypeErrors; update readUntil (the function named readUntil and
its local reader loop) to throw a descriptive Error when result.done is true and
the predicate hasn't matched, including the accumulated output in the error
message (e.g., "readUntil: stream ended without match: <output>") so callers
(such as the URL matching logic) get clear diagnostic information instead of a
null-match failure.
- Around line 272-310: The test "route matrix index/docs/guides" currently waits
only for URL_REGEX before making requests, which can be flaky; change the
readUntil call to wait for LAST_ROUTE_ENTRY (using readUntil(proc, text =>
LAST_ROUTE_ENTRY.test(text))) so the full route tree is printed first, then
extract the base URL by matching URL_REGEX against the accumulated output string
(the same output variable) and proceed with the three fetches; update references
in this test to use LAST_ROUTE_ENTRY and then derive baseUrl from the matched
URL_REGEX on output.
- Introduced a dedicated `linkNodeModules` function to streamline the creation of symlinks for the `node_modules` directory across multiple MDX test cases, enhancing code clarity and maintainability. - Added validation to ensure no stale keys are present in the expectations, improving test reliability. - Updated error handling in the `readUntil` function to provide clearer output when the stream ends unexpectedly. These changes contribute to a more robust and organized testing framework for MDX within the Bun ecosystem.
Contributor
Code ReviewNewest first 🔴 bcf78 — 1 issue(s) found
Powered by Claude Code Review |
Contributor
Code ReviewNewest first 🔴 d9489 — 2 issue(s) found
Note: An existing unresolved thread on line 260 already covers both enum mismatch issues (YAML and MDX loaders). No new inline comments posted to avoid duplication. Powered by Claude Code Review |
- Adjusted the BunLoaderType constants in headers-handwritten.h to correct the values for YAML, MD, and MDX loaders, ensuring proper functionality and consistency within the Bun framework. These changes enhance the accuracy of loader type definitions, contributing to improved module handling.
Contributor
Code ReviewNewest first 🔴 30a82 — 3 issue(s) found
Powered by Claude Code Review |
- Updated the MDX test case to assert that the exit code is not zero when a compilation error occurs, ensuring that the tests accurately reflect the expected behavior during error scenarios. - This change enhances the reliability of the test suite by properly validating error handling in the MDX compilation process.
Contributor
Code ReviewNewest first 🟡 373e1 — 1 issue found
Powered by Claude Code Review |
Contributor
Code ReviewNewest first ✅ f7548 — Looks good! Reviewed 32 files across Previous reviews🔴 5d96a — 2 issue(s) found
Powered by Claude Code Review |
… preserve expressions with closing braces in template literals. Introduced `StatementParseState` for better tracking of statement completion. Added tests to verify new functionality.
- Updated the argument validation in the `start` function to allow file-like arguments (e.g., glob patterns) alongside `.mdx` files. - Introduced a new check for file-like arguments and adjusted the logic to ensure proper resolution and handling of these inputs. - Ensured that duplicate resolved arguments are filtered out before processing. This change improves the flexibility of the argument parsing in the MDX processing workflow.
- Introduced a new function `parseHostnameAndPort` to enhance the parsing of hostnames and ports from command-line arguments. - Updated the `start` function to utilize this new parsing logic for `--hostname` and `--host` arguments, improving support for both bracketed and non-bracketed IPv6 addresses. - This change streamlines the argument handling process and increases robustness in the MDX processing workflow.
- Adjusted the argument handling logic in the `start` function to filter out duplicate resolved arguments after the loop, improving code clarity and maintainability. - This change ensures that the deduplication process is consistently applied regardless of the number of arguments, enhancing the robustness of the MDX processing workflow.
- Added a unique identifier to the temporary directory path used in the MDX processing workflow, improving the management of temporary files. - The temporary directory is now created in the system's temporary directory, ensuring better isolation and cleanup. - Updated the exit handler comment to clarify that SIGKILL cannot be handled during cleanup.
- Modified the `start` function to call `mdxInternal.start()` instead of `mdxInternal()`, aligning with the new export structure of the `mdx` module. - Updated the export of the `mdx` module to an object format, enhancing modularity and clarity in the codebase.
- Eliminated the handling of '{' and '}' characters in the JSXRenderer, simplifying the character writing logic.
- This change enhances code clarity and reduces complexity in the rendering process.
…ents - Simplified the logic for handling top-level import and export statements in the `extractTopLevelStatements` function. - Removed unnecessary variables and consolidated the statement completion checks, enhancing code clarity and maintainability. - Improved handling of multiline statements to ensure proper parsing and appending to the output.
…te literals - Added logic to skip braces inside line and block comments, improving expression parsing. - Enhanced handling of template literals and nested expressions, ensuring correct processing of embedded braces. - Updated tests to cover new functionality, including comments and escaped quotes within expressions.
- Introduced a new function to parse YAML frontmatter and serialize it as a JSON object, supporting nested structures and various data types. - Updated the `jsxImportSource` handling to accept undefined, null, and numbers, with appropriate coercion and validation. - Added comprehensive tests for frontmatter parsing and `jsxImportSource` behavior, ensuring robustness and correctness in the MDX compilation process.
- Updated the port assignment logic in both `html.ts` and `mdx.ts` to use pre-increment instead of post-increment, ensuring the correct port is assigned on retries. - This change improves the reliability of server port handling during startup.
Author
|
@Jarred-Sumner I methodically went through all of the Coderabbit-identified issues, and I have thoroughly tested this across many The initial thing I am looking for is confirmation this effort is on the right track and not a waste. Thank you! |
- Changed the environment check in the BundleV2 struct to always return true for Windows, ensuring consistent behavior across platforms. - This adjustment simplifies the logic related to environment handling in the bundler.
Author
|
…te logic - Added a new `writeRestoringExpressions` function to handle the restoration of expressions within JSX content, improving the handling of placeholders. - Refactored the `.normal` case in the `write` function to utilize the new expression restoration logic, enhancing code clarity and reducing duplication. - Updated the MDX compilation process to ensure unresolved placeholders are correctly identified and reported, improving error handling. - Introduced a complex fixture for testing, ensuring comprehensive coverage of frontmatter and expression handling in various scenarios.
Author
|
Commit: 06e4d7b — ProblemWhen MDX files are compiled, inline JavaScript expressions (like Two issues existed:
Solution1. Extract
|
| Expression | Evaluated Result |
|---|---|
{frontmatter.title} |
"Complex Frontmatter + MDX Parse Torture Test" (from YAML frontmatter) |
{frontmatter.description} |
The multi-line description string from frontmatter |
{summarizeChecks(parserMatrix.checks)} |
"3/3 checks enabled" (all 3 checks have enabled: true) |
{String(frontmatter.lastUpdated)} |
"2026-02-28" |
{numericSequence.map((n, idx) => ...)} |
All 8 Fibonacci items with correct parity (0=even, 1=odd, ..., 8=even, 13=odd) |
{(() => { ... })()} (IIFE at end) |
"Enabled checks: fm-title, jsx-inline, code-fence" |
Literal text (inside backticks / inline code)
Expressions inside backtick-delimited inline code are not evaluated — they render as literal text. This is controlled by the in_inline_code flag in replaceExpressions (mdx.zig, lines 392–399), which prevents {...} inside backticks from being treated as JSX expressions.
| Source | Renders As |
|---|---|
`{frontmatter.customerAbbr}` |
Literal {frontmatter.customerAbbr} |
`{2 ** 5}` |
Literal {2 ** 5} |
`{String(frontmatter.title)}` (table cells) |
Literal text, not evaluated |
`\{literal\}` |
Preserved escaped braces |
Structural elements
All of these render correctly through the standard markdown → JSX pipeline:
<InlineCard>— custom component renders itstitleprop and children<Fragment>— wraps thenumericSequence.map(...)list- Ordered/unordered lists — including nested bullets (alpha, beta, beta.one, beta.two)
- Blockquote — with bold, inline code, and line breaks
- Fenced code blocks — TypeScript and JSON samples preserved as-is
- Markdown table — rendered via
_components.tablewith literal expression text in cells
Test confirmation
The SSR runtime test (mdx.test.ts, line 378) validates this behavior end-to-end by importing the compiled MDX as a React component, rendering it server-side, and asserting:
"3/3 checks enabled"appears in the HTML"Enabled checks: fm-title, jsx-inline, code-fence"appears in the HTML- No
MDXEplaceholder strings leak into the output
Files Changed
| File | What Changed |
|---|---|
src/md/jsx_renderer.zig |
Extracted writeRestoringExpressions + ExprWriteMode enum; .normal and .html text types now delegate to it; unresolved placeholders now error instead of silently passing |
src/md/mdx.zig |
Added post-render check for \x01MDXE in output → error.UnresolvedPlaceholder |
test/js/bun/md/fixtures/mdx/complex-frontmatter.mdx |
New torture-test fixture |
test/js/bun/md/mdx.test.ts |
Three new tests (compile, SSR runtime, direct serve) + added fixture to existing fixture-iteration test |
…olution - Enhanced the MDX processing workflow to compile .mdx files to TSX in-memory, avoiding temporary file pollution and ensuring correct import resolution based on the original file location. - Introduced a Bun.plugin for handling .mdx file imports, allowing for automatic re-compilation on changes and improved hot module replacement (HMR) support. - Updated tests to verify that relative and sibling imports resolve correctly from the original file location, ensuring robust functionality in various scenarios.
Author
Fix: MDX Dev Server Module ResolutionCommitProblemWhen running
This only affected SolutionReplaced temp-file pre-compilation with an in-memory
Only minimal scaffolding (HTML shell + entry.js + node_modules symlink) remains in the temp directory. No files are written to the user's project. HMRThe Changes
Testing
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ref: Native First-Class MDX Support
Summary
Adds native MDX support to Bun —
.mdxfiles can be imported, bundled, transpiled, and served directly, just like.htmlfiles.Bun.mdx.compile(source, options?)— compile MDX source into a JSX module string. Supports frontmatter extraction, top-levelimport/exportstatements, inline{expressions}, and custom JSX components. Accepts strings, TypedArrays, and ArrayBuffers. Options extendBun.markdown.OptionswithjsxImportSource(default:"react")..mdxis a first-class loader (loader: "mdx"). MDX files can beimport-ed from.ts/.tsx/.jsfiles, used withBun.Transpiler, and processed by the bundler. The MDX compiler runs at parse time, producing JSX that feeds into the existing JS parser pipeline.bun file.mdx— serves MDX files via a dev server (same asbun index.html). Pre-compiles MDX to TSX, generates an HTML shell with a React mount point, and serves it withBun.serve()in development mode. Supports multi-file routing (bun ./*.mdx ./docs/*.mdx), glob deduplication,--port,--hostname, and--consoleflags..mdxsource files and re-compiles to the temp directory on change, so the dev server's HMR picks up edits automatically without needing--hot.Implementation details
src/md/mdx.zig) with JSX renderer (src/md/jsx_renderer.zig) — extracts frontmatter, top-level statements, and inline expressions, then renders markdown to JSX via the existing markdown parserMdxObject.zigexposesBun.mdx.compile()to JS usingbun.StringAPIsmdx = 21, integrated into ModuleLoader, ParseTask, LinkerContext, DevServer watch store, and the native bundler plugin APIsrc/js/internal/mdx.ts) handlesbun file.mdxserve mode withfs.watch()-based live reloadrun_command.zigroutes.mdxentry points through the HTML loader pathTest plan
Bun.mdx.compile()— basic compilation, frontmatter, expressions, TypedArray input,jsxImportSource, option aliases (hardSoftBreaks/hard_soft_breaks), invalid argument errors, malformed MDX errors, fixture documentsimportfrom.tsxentry point, import/export-heavy MDX, malformed MDX compile failure reportingBun.Transpilerwithloader: "mdx"--hostnamebinding