-
Notifications
You must be signed in to change notification settings - Fork 44
feat: assets copying #753
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
base: main
Are you sure you want to change the base?
feat: assets copying #753
Changes from all commits
c01a517
01f99f0
c7bea21
536ed59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| 'use strict'; | ||
|
|
||
| import { resolve, dirname, basename } from 'node:path'; | ||
|
|
||
| import { h as createElement } from 'hastscript'; | ||
| import { slice } from 'mdast-util-slice-markdown'; | ||
| import readingTime from 'reading-time'; | ||
|
|
@@ -287,6 +289,59 @@ export const createDocumentLayout = (entries, metadata) => | |
| }), | ||
| ]); | ||
|
|
||
| /** | ||
| * Checks if a given URL is a local asset path. | ||
| * | ||
| * @param {string} url - The URL or path to check. | ||
| * @returns {boolean} True if the asset is local, false otherwise. | ||
| */ | ||
| function isLocalAsset(url) { | ||
| if (!url || url.startsWith('//')) { | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| new URL(url); | ||
| return false; | ||
| } catch { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Traverses the AST of markdown files to find local image references, | ||
| * rewrites their URLs to a relative assets folder, and collects their paths. | ||
| * | ||
| * @param {Array<import('../../metadata/types').MetadataEntry>} metadataEntries - API documentation metadata entries | ||
| * @returns {Map<string, string>} A Map containing source paths as keys and destination paths as values. | ||
| */ | ||
| function extractAssetsFromAST(metadataEntries) { | ||
| const assetsMap = new Map(); | ||
| for (const entry of metadataEntries) { | ||
| if (!entry.content) { | ||
| continue; | ||
| } | ||
| visit(entry.content, 'image', imageNode => { | ||
| const originalUrl = imageNode.url; | ||
|
|
||
| if (isLocalAsset(originalUrl)) { | ||
| const sourceDir = entry.fullPath | ||
| ? dirname(entry.fullPath) | ||
| : process.cwd(); | ||
| const sourcePath = resolve(sourceDir, originalUrl); | ||
| const fileName = basename(originalUrl); | ||
|
|
||
| assetsMap.set(sourcePath, fileName); | ||
|
|
||
| // Rewrite AST URL | ||
| imageNode.url = `/assets/${fileName}`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| return assetsMap; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asset filename collisions silently overwrite different imagesMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 536ed59. Configure here. |
||
|
|
||
| /** | ||
| * @typedef {import('estree').Node & { data: import('../../metadata/types').MetadataEntry }} JSXContent | ||
| * | ||
|
|
@@ -296,6 +351,9 @@ export const createDocumentLayout = (entries, metadata) => | |
| * @returns {Promise<JSXContent>} | ||
| */ | ||
| const buildContent = async (metadataEntries, head) => { | ||
| // First extract assets and rewrite URLs in the AST | ||
| const assetsMap = Object.fromEntries(extractAssetsFromAST(metadataEntries)); | ||
|
|
||
| // The metadata is the heading without the node children | ||
| const metadata = omitKeys(head, [ | ||
| 'content', | ||
|
|
@@ -311,7 +369,7 @@ const buildContent = async (metadataEntries, head) => { | |
| const ast = await remark().run(root); | ||
|
|
||
| // The final MDX content is the expression in the Program's first body node | ||
| return { ...ast.body[0].expression, data: head }; | ||
| return { ...ast.body[0].expression, data: head, assetsMap }; | ||
| }; | ||
|
|
||
| export default buildContent; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,46 @@ | ||
| 'use strict'; | ||
|
|
||
| import { readFile } from 'node:fs/promises'; | ||
| import { readFile, cp, mkdir } from 'node:fs/promises'; | ||
| import { join } from 'node:path'; | ||
|
|
||
| import { processJSXEntries } from './utils/processing.mjs'; | ||
| import getConfig from '../../utils/configuration/index.mjs'; | ||
| import { writeFile } from '../../utils/file.mjs'; | ||
|
|
||
| /** | ||
| * Aggregates and copies local assets referenced in the markdown ASTs to the output directory. | ||
| * | ||
| * @param {Array<import('./types').JSXContent>} input - The processed entries containing asset maps | ||
| * @param {string} outputDir - The absolute path to the generation output directory | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async function copyProjectAssets(input, outputDir) { | ||
| const allAssets = new Map(); | ||
|
|
||
| for (const entry of input) { | ||
| if (entry.assetsMap) { | ||
| for (const [source, name] of Object.entries(entry.assetsMap)) { | ||
| allAssets.set(source, join(outputDir, 'assets', name)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (allAssets.size === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const assetsOutputDir = join(outputDir, 'assets'); | ||
| await mkdir(assetsOutputDir, { recursive: true }); | ||
|
|
||
| for (const [source, dest] of allAssets.entries()) { | ||
| try { | ||
| await cp(source, dest, { force: true }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
|
||
| } catch (err) { | ||
| console.error(`[doc-kit] Error copying asset: ${source}`, err); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Main generation function that processes JSX AST entries into web bundles. | ||
| * | ||
|
|
@@ -34,6 +68,8 @@ export async function generate(input) { | |
|
|
||
| // Write CSS bundle | ||
| await writeFile(join(config.output, 'styles.css'), css, 'utf-8'); | ||
|
|
||
| await copyProjectAssets(input, config.output); | ||
| } | ||
|
|
||
| return results.map(({ html }) => ({ html: html.toString(), css })); | ||
|
|
||


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.
Use
URL.parse()instead ofnew URL()Low Severity
isLocalAssetusesnew URL(url)inside a try/catch to detect local paths. The project convention prefersURL.parse(), which returnsnullfor invalid URLs instead of throwing. This can be simplified toreturn URL.parse(url) === null.Triggered by learned rule: Prefer URL.parse() and destructuring over new URL().property
Reviewed by Cursor Bugbot for commit 536ed59. Configure here.