Repository navigation
fix(portal): load an <img> in an .mdx page from beside the page and under a path #456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b4168a6
fix(portal): bundle the file an <img> in an .mdx page names
saeedjamshaid f9a7ec2
fix(portal): report an <img> in an .mdx page whose file is missing
saeedjamshaid 7f171b1
test(portal): build an .mdx page's <img> tags under a path
saeedjamshaid dd2e08c
refactor(portal): walk the <img> tags apart from rewriting them
saeedjamshaid 2082c49
fix(portal): read an <img>'s address without the spaces around it
saeedjamshaid b299b3c
fix(portal): trim only an <img>'s address, as remarkImage keeps a Mar…
saeedjamshaid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import path from 'node:path'; | ||
|
|
||
| /** An mdast node, as far as an `<img>` written in an `.mdx` page and the import added for its file go. */ | ||
| interface MarkdownNode { | ||
| type: string; | ||
| name?: string | null; | ||
| attributes?: JsxAttribute[]; | ||
| value?: unknown; | ||
| data?: unknown; | ||
| children?: MarkdownNode[]; | ||
| } | ||
|
|
||
| interface JsxAttribute { | ||
| type: string; | ||
| name?: string; | ||
| value?: unknown; | ||
| } | ||
|
|
||
| // 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 estreeProgram = (statement: object) => ({ type: 'Program', sourceType: 'module', body: [statement] }); | ||
|
|
||
| /** Bundles the file an `<img>` names, as `remarkImage` does for Markdown images alone. page.ts checks the same. */ | ||
| export function remarkJsxImages({ publicDir }: { publicDir: string | false }) { | ||
| return (tree: MarkdownNode, { dirname }: { dirname?: string }) => { | ||
| if (dirname === undefined) return; | ||
| const imports: MarkdownNode[] = []; | ||
| for (const { attribute, address } of imageSources(tree)) { | ||
| const file = fileOf(address, publicDir, dirname); | ||
| if (file === null) continue; | ||
| // As `remarkImage` writes it, so a file on another drive is `./C:/…`, which static-images.ts resolves. | ||
| const relative = path.relative(dirname, file).replaceAll(path.sep, '/'); | ||
| const name = `__jsxImg${imports.length}`; | ||
| imports.push(importOf(name, relative.startsWith('../') ? relative : `./${relative}`)); | ||
| attribute.value = expressionOf(name); | ||
| } | ||
| tree.children?.unshift(...imports); | ||
| }; | ||
| } | ||
|
|
||
| function* imageSources(node: MarkdownNode): Generator<{ attribute: JsxAttribute; address: string }> { | ||
| if ((node.type === 'mdxJsxFlowElement' || node.type === 'mdxJsxTextElement') && node.name === 'img') { | ||
| const src = node.attributes?.find(({ type, name }) => type === 'mdxJsxAttribute' && name === 'src'); | ||
| // A browser drops the spaces around an `<img>`'s address; `remarkImage` keeps a Markdown image's. | ||
| if (typeof src?.value === 'string') yield { attribute: src, address: src.value.trim() }; | ||
| } | ||
| for (const child of node.children ?? []) yield* imageSources(child); | ||
| } | ||
|
|
||
| function fileOf(url: string, publicDir: string | false, dirname: string): string | null { | ||
| const address = url.split(/[?#]/, 1)[0]; | ||
| if (address.length === 0 || address.endsWith('/') || NOT_A_FILE.test(address)) return null; | ||
| const decoded = decodedAddress(address); | ||
| if (!decoded.startsWith('/')) return path.join(dirname, decoded); | ||
| return publicDir === false ? null : path.join(publicDir, decoded); | ||
| } | ||
|
|
||
| function decodedAddress(address: string): string { | ||
| try { | ||
| return decodeURI(address); | ||
| } catch { | ||
| return address; | ||
| } | ||
| } | ||
|
|
||
| function importOf(name: string, source: string): MarkdownNode { | ||
| return { | ||
| type: 'mdxjsEsm', | ||
| value: `import ${name} from ${JSON.stringify(source)}`, | ||
| data: { | ||
| estree: estreeProgram({ | ||
| type: 'ImportDeclaration', | ||
| source: { type: 'Literal', value: source }, | ||
| specifiers: [{ type: 'ImportDefaultSpecifier', local: { type: 'Identifier', name } }] | ||
| }) | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| function expressionOf(name: string): MarkdownNode { | ||
| return { | ||
| type: 'mdxJsxAttributeValueExpression', | ||
| value: name, | ||
| data: { estree: estreeProgram({ type: 'ExpressionStatement', expression: { type: 'Identifier', name } }) } | ||
| }; | ||
| } |
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| import path from 'node:path'; | ||
| import { expect } from 'chai'; | ||
| import { remark } from 'remark'; | ||
| import remarkMdx from 'remark-mdx'; | ||
| import { FileName } from '../../src/types/file/fileName'; | ||
| import { parsePage } from '../../src/types/portal/page'; | ||
| import { remarkJsxImages } from '../../portal-template/remark-jsx-images'; | ||
|
|
||
| describe('the <img> tags of the .mdx content pages', () => { | ||
| const project = path.resolve('/portal'); | ||
| const staticDirectory = path.join(project, 'static'); | ||
| const pageDirectory = path.join(project, 'content', 'guides'); | ||
| const processor = remark().use(remarkMdx); | ||
|
|
||
| /** The page once the step has run, written back as MDX, the imports it adds at the top. */ | ||
| const transformed = (mdx: string, publicDir: string | false = staticDirectory): string => { | ||
| const tree = processor.parse(mdx); | ||
| remarkJsxImages({ publicDir })(tree, { dirname: pageDirectory }); | ||
| return processor.stringify(tree); | ||
| }; | ||
|
|
||
| it('imports the file beside the page or in the static directory, keeping the other attributes', () => { | ||
| expect(transformed('<img src="./flow.png" width="200" alt="Flow" />\n\n<img src="/images/logo.png" />\n')).to.equal( | ||
| 'import __jsxImg0 from "./flow.png"\n\nimport __jsxImg1 from "../../static/images/logo.png"\n\n' + | ||
| '<img src={__jsxImg0} width="200" alt="Flow" />\n\n<img src={__jsxImg1} />\n' | ||
| ); | ||
| }); | ||
|
|
||
| it('imports an <img> inside a line of text, and one inside a component', () => { | ||
| expect( | ||
| transformed('Inline <img src="../shared.png" /> here.\n\n<Card>\n <img src="diagram.png" />\n</Card>\n') | ||
| ).to.equal( | ||
| 'import __jsxImg0 from "../shared.png"\n\nimport __jsxImg1 from "./diagram.png"\n\n' + | ||
| 'Inline <img src={__jsxImg0} /> here.\n\n<Card>\n <img src={__jsxImg1} />\n</Card>\n' | ||
| ); | ||
| }); | ||
|
|
||
| it('reads the address without the spaces around it or its query or fragment, and decoded', () => { | ||
| expect(transformed('<img src=" ./my%20flow.png?v=2#top " />\n')).to.equal( | ||
| 'import __jsxImg0 from "./my flow.png"\n\n<img src={__jsxImg0} />\n' | ||
| ); | ||
| }); | ||
|
|
||
| const namingNoFile = [ | ||
| '<img src="https://example.com/a.png" />', | ||
| '<img src="//cdn.example.com/a.png" />', | ||
| '<img src="data:image/png;base64,AAAA" />', | ||
| '<img src="" />', | ||
| '<img src="images/" />', | ||
| '<img src={logo} />', | ||
| '<img alt="No address" />' | ||
| ]; | ||
|
|
||
| it('leaves an address that names no file as written, and an <img> without one', () => { | ||
| const mdx = namingNoFile.join('\n\n') + '\n'; | ||
|
|
||
| expect(transformed(mdx)).to.equal(mdx); | ||
| }); | ||
|
|
||
| it('leaves other tags as written, a component named Img among them', () => { | ||
| const mdx = '<Img src="./a.png" />\n\n<image src="./a.png" />\n\n<video src="./a.mp4" />\n'; | ||
|
|
||
| expect(transformed(mdx)).to.equal(mdx); | ||
| }); | ||
|
|
||
| it('leaves an address in the static directory as written when there is none', () => { | ||
| expect(transformed('<img src="/images/logo.png" />\n\n<img src="./flow.png" />\n', false)).to.equal( | ||
| 'import __jsxImg0 from "./flow.png"\n\n<img src="/images/logo.png" />\n\n<img src={__jsxImg0} />\n' | ||
| ); | ||
| }); | ||
|
|
||
| // The step runs on every page of the author's; in a `.md` page a tag is raw HTML, which the build leaves out. | ||
| it("leaves a .md page's <img> as written", () => { | ||
| const markdown = remark(); | ||
| const tree = markdown.parse('<img src="./flow.png">\n'); | ||
| remarkJsxImages({ publicDir: staticDirectory })(tree, { dirname: pageDirectory }); | ||
|
|
||
| expect(markdown.stringify(tree)).to.equal('<img src="./flow.png">\n'); | ||
| }); | ||
|
|
||
| it('leaves a page it has no path for as written', () => { | ||
| const tree = processor.parse('<img src="./flow.png" />\n'); | ||
| remarkJsxImages({ publicDir: staticDirectory })(tree, {}); | ||
|
|
||
| expect(processor.stringify(tree)).to.equal('<img src="./flow.png" />\n'); | ||
| }); | ||
|
|
||
| // MDX compiles these programs, not the text the tests above read. | ||
| it('gives MDX the program of each import and of the address', () => { | ||
| const tree = processor.parse('<img src="./flow.png" />\n'); | ||
| remarkJsxImages({ publicDir: staticDirectory })(tree, { dirname: pageDirectory }); | ||
| const [esm, image] = tree.children as unknown as [ | ||
| { data: { estree: unknown } }, | ||
| { attributes: [{ value: { data: { estree: unknown } } }] } | ||
| ]; | ||
|
|
||
| expect(esm.data.estree).to.deep.equal({ | ||
| type: 'Program', | ||
| sourceType: 'module', | ||
| body: [ | ||
| { | ||
| type: 'ImportDeclaration', | ||
| source: { type: 'Literal', value: './flow.png' }, | ||
| specifiers: [{ type: 'ImportDefaultSpecifier', local: { type: 'Identifier', name: '__jsxImg0' } }] | ||
| } | ||
| ] | ||
| }); | ||
| expect(image.attributes[0].value.data.estree).to.deep.equal({ | ||
| type: 'Program', | ||
| sourceType: 'module', | ||
| body: [{ type: 'ExpressionStatement', expression: { type: 'Identifier', name: '__jsxImg0' } }] | ||
| }); | ||
| }); | ||
|
|
||
| // 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 () => { | ||
| const mdx = | ||
| [ | ||
| '---\ntitle: Images\n---', | ||
| '<img src="/images/a%20b.png?v=1" />', | ||
| 'Inline <img src="./c.png#top" /> here.', | ||
| '<Card>\n <img src="../d.png" />\n</Card>', | ||
| '<img src=" ./g.png " />', | ||
| '<img src="./h%E0.png" />', | ||
| ...namingNoFile | ||
| ].join('\n\n') + '\n'; | ||
| const checked = (await parsePage(mdx, new FileName('page.mdx'), 'content/guides/page.mdx')).images; | ||
| const imported = (publicDir: string | false) => | ||
| [...transformed(mdx, publicDir).matchAll(/^import __jsxImg\d+ from "(.+)"$/gm)].map(([, source]) => | ||
| path.join(pageDirectory, source) | ||
| ); | ||
|
|
||
| expect(imported(staticDirectory)).to.deep.equal( | ||
| checked.map(({ from, path: file }) => path.join(from === 'static' ? staticDirectory : pageDirectory, file)) | ||
| ); | ||
| // Without a static directory the CLI refuses a `/…` image as missing, so the two need to agree only on the rest. | ||
| expect(imported(false)).to.deep.equal( | ||
| checked.flatMap(({ from, path: file }) => (from === 'page' ? [path.join(pageDirectory, file)] : [])) | ||
| ); | ||
| }); | ||
|
|
||
| it('imports a static file on another drive by its own path, which static-images.ts resolves', function () { | ||
| if (process.platform !== 'win32') this.skip(); | ||
|
|
||
| const tree = processor.parse('<img src="/images/logo.png" />\n'); | ||
| remarkJsxImages({ publicDir: 'D:\\static' })(tree, { dirname: 'C:\\portal\\content' }); | ||
|
|
||
| expect(processor.stringify(tree)).to.equal( | ||
| 'import __jsxImg0 from "./D:/static/images/logo.png"\n\n<img src={__jsxImg0} />\n' | ||
| ); | ||
| }); | ||
| }); | ||
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
decodeURIrejects (./a%E0.png);/;data:and//addresses;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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
./h%E0.png, the "names no file" list (now shared by both tests), and a run withpublicDir: false, where only the files beside the page are imported.