Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/tidy-melons-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@cloudflare/nimbus-docs": patch
---

Fix authored-link normalization failing on JSX elements whose TypeScript parse drifts from the mdast node range.

Match JSX elements by their start offset instead of their full range, skip elements without `href` attributes before the TypeScript round-trip, and reconcile attributes by name instead of index. This stops "ambiguous JSX range"/"ambiguous JSX attributes" failures on multi-line elements whose children contain JSX-like tokens (for example `{` inside a code block) or whose raw slice spans blockquote markers.

Also skip normalization for plain `.md` files, which are not guaranteed to be valid MDX (legacy files can contain HTML comments or prose with `{key: value}`) and previously failed the build on an MDX parse error.
68 changes: 36 additions & 32 deletions packages/nimbus-docs/src/_internal/authored-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ interface ParsedJsxRange {
sourceBase: number;
}

function jsxRangeKey(start: number, end: number): string {
return `${start}:${end}`;
function jsxStartKey(start: number): string {
return `${start}`;
}

function staticHrefOffsets(
Expand All @@ -256,7 +256,16 @@ function staticHrefOffsets(
if (!Array.isArray(node.attributes)) {
fail("missing JSX attributes", source, sourceId, sourceStart);
}
const key = jsxRangeKey(sourceStart, sourceStart + raw.length);
const hasHref = (
node.attributes as Array<{ type?: string; name?: unknown }>
).some(
(attribute) =>
attribute?.type === "mdxJsxAttribute" &&
typeof attribute.name === "string" &&
isHref(node, attribute.name),
);
if (!hasHref) return [];
const key = jsxStartKey(sourceStart);
if (!parsedRanges.has(key)) {
const prefix = "const element = (";
const parsed = ts.createSourceFile(
Expand All @@ -273,13 +282,11 @@ function staticHrefOffsets(
ts.isJsxSelfClosingElement(candidate) ||
ts.isJsxFragment(candidate)
) {
parsedRanges.set(
jsxRangeKey(
candidate.getStart(parsed) + sourceBase,
candidate.getEnd() + sourceBase,
),
{ node: candidate, sourceFile: parsed, sourceBase },
);
parsedRanges.set(jsxStartKey(candidate.getStart(parsed) + sourceBase), {
node: candidate,
sourceFile: parsed,
sourceBase,
});
}
ts.forEachChild(candidate, collect);
};
Expand All @@ -299,12 +306,9 @@ function staticHrefOffsets(
const properties = ts.isJsxElement(element)
? element.openingElement.attributes.properties
: element.attributes.properties;
if (properties.length !== node.attributes.length) {
fail("ambiguous JSX attributes", source, sourceId, sourceStart);
}
const offsets: number[] = [];

for (const [index, value] of node.attributes.entries()) {
for (const value of node.attributes) {
if (!value || typeof value !== "object") {
fail("invalid JSX attribute", source, sourceId, sourceStart);
}
Expand All @@ -313,32 +317,27 @@ function staticHrefOffsets(
name?: unknown;
value?: unknown;
};
const property = properties[index]!;

if (attribute.type === "mdxJsxExpressionAttribute") {
if (!ts.isJsxSpreadAttribute(property)) {
fail(
"ambiguous JSX spread expression",
source,
sourceId,
property.getStart(parsed) + sourceBase,
);
}
continue;
}

if (
attribute.type !== "mdxJsxAttribute" ||
typeof attribute.name !== "string" ||
!ts.isJsxAttribute(property) ||
property.name.getText(parsed) !== attribute.name
typeof attribute.name !== "string"
) {
fail(
"unsupported JSX attribute",
source,
sourceId,
property.getStart(parsed) + sourceBase,
);
fail("unsupported JSX attribute", source, sourceId, sourceStart);
}
const property = properties.find(
(candidate) =>
ts.isJsxAttribute(candidate) &&
candidate.name.getText(parsed) === attribute.name,
);
if (!property || !ts.isJsxAttribute(property)) {
// The TypeScript parse could not reconcile this attribute with the
// mdast node (for example, the raw slice spans blockquote markers that
// are not valid JSX). Skip it rather than failing the whole source.
continue;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only normalize href on native elements? If a native html cannot be read, might be worth returning an error instead of skipping it.

}
if (attribute.value === null) {
if (property.initializer) {
Expand Down Expand Up @@ -416,6 +415,11 @@ export function normalizeAuthoredLinks(
try {
tree = mdxToMdast(source) as MdNode;
} catch (error) {
// Plain `.md` files are not guaranteed to be valid MDX (for example,
// legacy files with HTML comments or prose containing `{key: value}`).
// Leave them untouched rather than failing the build on a parse error.
// `.mdx` sources still fail closed.
if (options.sourceId?.endsWith(".md")) return source;
const detail = error instanceof Error ? error.message : String(error);
const location = detail.match(/^(\d+):(\d+):\s*/);
if (location) {
Expand Down
59 changes: 59 additions & 0 deletions packages/nimbus-docs/test/authored-links.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,62 @@ test("rejects an invalid deployment base", () => {
);
}
});

test("normalizes hrefs on JSX elements whose children contain JSX-like tokens", () => {
const source = `<Tabs syncKey="x" href="/guide">

\`\`\`js
{
\`\`\`

</Tabs>`;
assert.equal(
normalizeAuthoredLinks(source, { base: "/docs" }),
`<Tabs syncKey="x" href="/docs/guide">

\`\`\`js
{
\`\`\`

</Tabs>`,
);
});

test("skips JSX elements whose raw slice spans blockquote markers", () => {
const source = `> Example:
> <PackageManagers
> \ttype="create"
> \tpkg="vike@latest"
> />`;
assert.equal(normalizeAuthoredLinks(source, { base: "/docs" }), source);
});

test("skips blockquote-mangled JSX hrefs instead of failing", () => {
const source = `> <Card
> \thref="/card"
> />`;
assert.equal(normalizeAuthoredLinks(source, { base: "/docs" }), source);
});

test("leaves plain Markdown files that are not valid MDX untouched", () => {
const source = `<!-- Auto Generated Below -->

<a name="module_x"></a>

[Guide](/guide)`;
assert.equal(
normalizeAuthoredLinks(source, { base: "/docs", sourceId: "generated.md" }),
source,
);
});

test("still normalizes links in Markdown files that are valid MDX", () => {
const source = `[Guide](/guide)`;
assert.equal(
normalizeAuthoredLinks(source, {
base: "/docs",
sourceId: "generated.md",
}),
`[Guide](/docs/guide)`,
);
});
Loading