Repository navigation
fix(portal): load an <img> in an .mdx page from beside the page and under a path - #456
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the plugin to the template root, beside the other build modules that use Node's APIs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mehnoorsiddiqui
left a comment
There was a problem hiding this comment.
Thanks, this is a solid fix. I tested it on Windows:
- the full suite with
APIMATIC_E2E=1; - a 24-case
.mdxpage built under/apiand loaded in headless Chrome, with every request logged; vite dev, including an<img>and its file added while the server runs;- a static directory on another drive;
- a check of which file the plugin and the CLI pick for about 60 tricky addresses: the two agreed every time.
Every local <img> loads from /api/assets/…, the attributes are kept, and the Markdown copies stay as written.
One change before merge (inline on page.ts): the new trim() also applies to Markdown images, so the CLI now accepts a Markdown image the build then fails on.
Not inline, worth deciding on:
- An
<img>in an.mdxfile pulled in with<include>now fails the whole build. The CLI check doesn't catch it. Ondevthe same page built with a broken image; here the build stops withCould not resolve './inner.png' in content/includer.mdx. It's listed under "Not in this PR", but for these portals it changes a broken image into a failed build. Either check included files in the CLI, or call it out in the release notes. - An
<img>inside a component's prop isn't covered, e.g.<Card icon={<img src="./a.png" />} />. Neither the plugin nor the check sees it, so it still 404s under a path. Worth adding to "Not in this PR". - Root-hosted builds: the issue's "How to verify" says images should load "under a path and at the root". Only the
/apifixture covers<img>tags now. A root build worked when I tried it by hand, but nothing tests it. - Commit
2082c49dboth fixes the trim and moves the file, which makes it harder to bisect. Minor.
The other inline comments are small maintainability suggestions; none of them blocks the merge.
| // A browser drops the spaces around an `<img>`'s address. | ||
| const address = url.trim().split(/[?#]/, 1)[0]; |
There was a problem hiding this comment.
The trim now applies to Markdown images too, and Fumadocs' remarkImage doesn't trim. It calls parseSrc(decodeURI(node.url), …) as written.
Here's what happens with  in a .md page, where the spaces inside the angle brackets are part of the URL:
- On
dev, the CLI refuses the page and names it. - With this PR, the CLI accepts it, and then the build fails with
[UNRESOLVED_IMPORT] Could not resolve './ ./flow.png ' in content/trim.md.
I suggest trimming only the <img> address, for example return node.name === 'img' && typeof src === 'string' ? src.trim() : undefined; in addressOf. Then located stays as it was for Markdown images.
PageImage.url should probably keep the untrimmed value, since the error message quotes it as written. If so, pass the trimmed value through separately rather than trimming in addressOf.
There was a problem hiding this comment.
Fixed. Only an <img>'s src is trimmed now, in addressOf, and located reads a Markdown image's address as remarkImage does; a test covers . I kept url trimmed for an <img>, since the message gives the line and spaces inside quotes are hard to see.
| return node.url; | ||
| case 'imageReference': | ||
| return definitions.get(node.identifier ?? ''); | ||
| // An `<img>` in an `.mdx` page, which the template imports too (remark-jsx-images.ts). |
There was a problem hiding this comment.
Nit: the case labels already say the first half of this comment. The pointer to remark-jsx-images.ts is the part worth keeping, e.g. // remark-jsx-images.ts imports these too.
There was a problem hiding this comment.
Cut to the pointer.
|
|
||
| function fileOf(address: string, publicDir: string | false, dirname: string): string | null { | ||
| // A browser drops the spaces around an address. | ||
| const written = address.trim().split(/[?#]/, 1)[0]; |
There was a problem hiding this comment.
This is the template's copy of the rule in page.ts (located/decodedAddress). The parity test keeps the two copies agreeing on behaviour. Making them read the same would also let a plain diff show any drift. Right now they differ in small ways:
- the names:
writtenhere,addressthere; - the comment wording for the trim;
locatedstrips leading/s with a regex, while this copy relies onpath.join;page.tsreadssrcbefore it checksname === 'img', andimageSourceschecks the name first.
None of these is a bug.
There was a problem hiding this comment.
Aligned the names (url, address, decoded), and both copies now trim where src is read, under the same comment. Two differences stay: the CLI needs the path inside static/ while the plugin joins it to the directory, and one copy is a switch case while the other is a generator.
| return (tree: MarkdownNode, { dirname }: { dirname?: string }) => { | ||
| if (dirname === undefined) return; | ||
| const imports: MarkdownNode[] = []; | ||
| for (const [src, address] of imageSources(tree)) { |
There was a problem hiding this comment.
This loop still does three things: works out the file and its relative path, builds the mdxjsEsm import node, and rewrites src. Pulling out something like importOf(name, source), beside a small expressionOf(name), would finish the separation that dd2e08c started. The loop would then read as: resolve, import, replace.
There was a problem hiding this comment.
Done, with importOf and expressionOf.
| // A scheme, a drive letter or a network path: none of them names a file the build imports. | ||
| const NOT_A_FILE = /^(?:[a-z][a-z\d+.-]*:|\/\/)/i; | ||
|
|
||
| const program = (statement: object) => ({ type: 'Program', sourceType: 'module', body: [statement] }); |
There was a problem hiding this comment.
Nit: program could say a little more about what it builds, e.g. estreeProgram(statement) or moduleOf(statement).
There was a problem hiding this comment.
Renamed to estreeProgram.
| }; | ||
| } | ||
|
|
||
| function* imageSources(node: MarkdownNode): Generator<[JsxAttribute, string]> { |
There was a problem hiding this comment.
Nit: the string in the tuple is always src.value already narrowed. You could yield only the attribute and read src.value as string at the call site. Or keep the tuple and name it, e.g. { attribute, address }.
There was a problem hiding this comment.
It yields { attribute, address } now.
| }); | ||
|
|
||
| // The CLI reports a missing file before a build that would otherwise fail over its import, naming no page. | ||
| it('imports exactly the files the CLI checks', async () => { |
There was a problem hiding this comment.
This is the test that would catch the two copies drifting, but its input avoids the edge cases where they're most likely to drift. Consider adding:
- an address
decodeURIrejects (./a%E0.png); - one ending in
/; data:and//addresses;- a run with
publicDir: false.
Reusing the inputs of the "leaves an address that names no file" test would do most of it.
There was a problem hiding this comment.
Added ./h%E0.png, the "names no file" list (now shared by both tests), and a run with publicDir: false, where only the files beside the page are imported.
| ); | ||
| }); | ||
|
|
||
| (process.platform === 'win32' ? it : it.skip)( |
There was a problem hiding this comment.
Nit: the other Windows-only tests skip with if (process.platform !== 'win32') this.skip();, for example test/types/file/directory-path.test.ts:35, test/actions/portal/prepare-project.test.ts:21 and test/portal-template/static-images.test.ts:98. Using that form here would match them.
There was a problem hiding this comment.
Switched to this.skip().
| import { prerenderPages } from './prerender-pages.ts'; | ||
| import { specReload } from './spec-reload.ts'; | ||
| import { remarkImageReferences } from './src/lib/remark-image-references.ts'; | ||
| import { remarkJsxImages } from './remark-jsx-images.ts'; |
There was a problem hiding this comment.
Nit: the local imports are otherwise in alphabetical order, so this one belongs after ./prerender-pages.ts.
…kdown image's spaces The CLI trimmed a Markdown image's address too, so it accepted `` while the build looked for `./ ./flow.png ` and failed. Both copies of the rule now trim where an <img>'s src is read. Also from the review: the plugin's import and address nodes move into importOf and expressionOf, the copies share their names, the contract test covers the addresses the plugin leaves alone, one decodeURI refuses and a portal without a static directory, and the root-hosted fixture bundles an <img> beside its page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
Thanks for testing it this thoroughly. On the points outside the code:
|



The problem
An
<img>in an.mdxpage that names a local file doesn't load:<img src="./flow.png" />is left as written, so the browser asks forflow.pngbeside the page's address, where the build never writes it.<img src="/images/logo.png" />loads at the root of a host. A portal hosted under a path, such ashttps://docs.example.com/api, serves the file at/api/images/logo.png, so the image is missing.<img>that names a file that doesn't exist builds without a word and shows a broken image.A Markdown image,
, has none of these problems, because Fumadocs bundles it. Fumadocs bundles Markdown images only, and an<img>is the usual way to set an image's width.Part of apimatic-io issue 2314.
What changes
<img>that names a local file is bundled like a Markdown image.remark-jsx-images.ts, imports the file and setssrcto the import.srcby the rule Markdown images follow:/…fromsrc/static/, anything else from beside the page, without its query or fragment, decoded. As a browser does, it drops the spaces around the address.widthandalt, are kept.//ordata:address,src={…}, and an<img>in a.mdpage, whose HTML the build leaves out.<img>names a missing file used to build; it is now refused. That includes an<img>that loaded only because of how the browser resolved its address:images/logo.pngon a top-level page, at the root of a host, forstatic/images/logo.png. Write it as/images/logo.png./…address the portal doesn't serve, such as/api/images/logo.pngwith the path written by hand, or a file another app serves on the same host. Write the first as/images/logo.png, and the second as a fullhttps://address.<img>in a file pulled in with<include>can now stop the build. It is looked for beside the page that includes the file, as a Markdown image there already is. When its file isn't there, the build fails withCould not resolve './inner.png' in content/includer.mdx, where it used to build with a broken image. The CLI doesn't check included files, for either kind of image. Worth a line in the release notes.remark-image-references.test.tsdoes for reference-style images. The template can't import the CLI's code, so the rule is written in both.The Markdown copies and
llms-full.txtstill show the tags as written.Testing
pnpm testwithAPIMATIC_E2E=1, as CI runs it: 2056 passing, ESLint clean.<img>in a line of text and in a component, the spaces, query, fragment and encoding, each address it leaves alone, a.mdpage, and, on Windows, a static directory on another drive;<img>tags it reads, the spaces around an<img>'s address, which a Markdown image keeps as the build does, and a missing file refused with its page and line;decodeURIrefuses. It fails when the CLI is made to skip an<img>inside a line of text, or the plugin to import a static file with no static directory./apigainsfigures.mdx, with an<img>fromstatic/and one from beside the page. Both load from/api/assets/…, thewidthis kept, the page names nothing on its host outside/api, and its Markdown copy keeps the tags. In the fixture hosted at the root, an<img>naming a file beside its page is bundled too./apiand opened in headless Chrome. All the images load, with no failed requests. Once an<img>naming a missing file is added, the checkportal serveruns on a save refuses it, with the page and line.Not in this PR
.mdpage's HTML and components: the build still leaves them out without a word. Naming them in a warning is the second PR for apimatic-io issue 2314.<img>in the generated SDK pages, and other tags that name files:srcset,<picture>'s<source>,<video src>, and a raw<a href="/…">.<img>written inside a prop, such as<Card icon={<img src="./a.png" />} />. It's code rather than a tag of the page, so neither the plugin nor the check sees it, and it is left as written, as before.<img>keeps its own look. A Markdown image gets the portal's rounded corners and responsive sizes; an<img>doesn't, as before.<img src="/images/icons.svg#check" />shows the whole SVG instead of itscheckview.<img>now has too:.heic,.tiffor one without an extension, fails the build without naming the page;\images\a.pngis refused as pointing outsidecontent/.🤖 Generated with Claude Code