feat: assets copying by moshams272 · Pull Request #753 · nodejs/doc-kit

moshams272 · 2026-04-05T19:10:42Z

Description

Local images referenced in Markdown files were not resolved correctly or copied to the final build, this PR will solve this by copying the images in the output/assets/.

I solved this from root from the jsx-ast phase as they are markdown AST because we can iterate on them easier than JSX elements.

Steps:

Updated src/generators/ast/generate.mjs to include the absolute fullPath of each Markdown file.
Updated src/generators/metadata/utils/parse.mjs to include the fullPath into the metadata entries.
Implemented extractAssetsFromAST in src/generators/jsx-ast/utils/buildContent.mjs, that identifies local images, rewrites their URLs to a centralized /assets/ directory, and collects the source paths.
Implemented copyProjectAssets to the web generator src/generators/web/generate.mjs to aggregate all unique assets and copy them to the output directory.

Validation

Added tests in src/generators/jsx-ast/utils/__tests__/buildContent.test.mjs to verify that the assetsMap is populated with absolute paths.
Tested against Node.js API documentation; local images (like logo.png) are now correctly copied to the out/assets folder and displayed in the web generated.

Related Issues

#748

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run node --run test and all tests passed.
I have check code formatting with node --run format & node --run lint.
I've covered new added functionality with unit tests if necessary.

cursor · 2026-04-05T19:10:49Z

PR Summary

Medium Risk
Adds new path-based asset rewriting and filesystem copy behavior during generation; mistakes could break image URLs or cause missing/incorrectly overwritten assets in the output.

Overview
Adds support for local markdown image assets in generated web docs by threading each markdown file’s absolute fullPath through AST/metadata generation, rewriting local image URLs to /assets/<filename> during buildContent, and returning an assetsMap alongside the JSX AST.

The web generator now aggregates these asset maps and copies the referenced files into output/assets/ during build, with a new unit test asserting local URL resolution and asset-map population.

^{Reviewed by Cursor Bugbot for commit 536ed59. Bugbot is set up for automated code reviews on this repo. Configure here.}

vercel · 2026-04-05T19:10:50Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
api-docs-tooling	Ready	Preview	Apr 5, 2026 7:11pm

codecov · 2026-04-05T19:11:40Z

Codecov Report

❌ Patch coverage is 63.33333% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.42%. Comparing base (2680e84) to head (536ed59).

Files with missing lines	Patch %	Lines
src/generators/web/generate.mjs	0.00%	37 Missing ⚠️
src/generators/jsx-ast/utils/buildContent.mjs	88.13%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #753      +/-   ##
==========================================
+ Coverage   77.08%   77.42%   +0.33%     
==========================================
  Files         153      153              
  Lines       13749    13865     +116     
  Branches     1111     1122      +11     
==========================================
+ Hits        10599    10735     +136     
+ Misses       3146     3126      -20     
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cursor

Cursor Bugbot has reviewed your changes and found 4 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 536ed59. Configure here.}

cursor · 2026-04-05T19:18:22Z

src/generators/jsx-ast/utils/buildContent.mjs

+  }
+
+  return assetsMap;
+}


Asset filename collisions silently overwrite different images

Medium Severity

extractAssetsFromAST uses basename(originalUrl) as the destination filename, so two different images with the same name from different directories (e.g., ./diagrams/arch.png in fs.md and ./diagrams/arch.png in net.md) both map to arch.png in /assets/. One copy silently overwrites the other, and both markdown files end up referencing the wrong image. The AST URLs are also both rewritten to /assets/arch.png, making the collision unrecoverable.

Additional Locations (1)

src/generators/web/generate.mjs#L16-L41

^{Reviewed by Cursor Bugbot for commit 536ed59. Configure here.}

cursor · 2026-04-05T19:18:22Z

src/generators/web/generate.mjs

+
+  for (const [source, dest] of allAssets.entries()) {
+    try {
+      await cp(source, dest, { force: true });


Use copyFile with COPYFILE_FICLONE for files

Low Severity

cp is used to copy individual asset files, but the project convention requires copyFile from node:fs/promises with the constants.COPYFILE_FICLONE flag for single-file copies. copyFile with COPYFILE_FICLONE attempts a zero-copy clone and gracefully falls back, which is more efficient for single files than cp.

^{Triggered by learned rule: Use copyFile with COPYFILE_FICLONE instead of manual readFile+writeFile}

^{Reviewed by Cursor Bugbot for commit 536ed59. Configure here.}

cursor · 2026-04-05T19:18:22Z

src/generators/jsx-ast/utils/buildContent.mjs

+    return false;
+  } catch {
+    return true;
+  }


Use URL.parse() instead of new URL()

Low Severity

isLocalAsset uses new URL(url) inside a try/catch to detect local paths. The project convention prefers URL.parse(), which returns null for invalid URLs instead of throwing. This can be simplified to return 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.}

cursor · 2026-04-05T19:18:22Z

src/generators/jsx-ast/utils/buildContent.mjs

+        assetsMap.set(sourcePath, fileName);
+
+        // Rewrite AST URL
+        imageNode.url = `/assets/${fileName}`;


Hardcoded 'assets' path duplicated across two generators

Low Severity

The string 'assets' is hardcoded as an output directory name in both buildContent.mjs (URL rewrite to `/assets/${fileName}`) and generate.mjs (filesystem path via join(outputDir, 'assets', name)). Since this is a cross-generator constant controlling output path structure, it belongs as a named constant. Changing it in one generator without the other would silently break image resolution.

Additional Locations (1)

src/generators/web/generate.mjs#L22-L23

^{Triggered by learned rule: Regex patterns and external URLs belong in constants files}

^{Reviewed by Cursor Bugbot for commit 536ed59. Configure here.}

avivkeller · 2026-04-05T19:23:49Z

Why can't the user specify "all my assets are in /xyz/, copy that", this seems like a lot of work and data that needs to now be processed

moshams272 · 2026-04-05T19:35:56Z

Why can't the user specify "all my assets are in /xyz/, copy that", this seems like a lot of work and data that needs to now be processed

If you mean as staticDir in the config file, I think when a project uses doc-kit he cann't iterate to put them in one folder!

what do you think?

avivkeller · 2026-04-05T19:46:05Z

I think when a project uses doc-kit he cann't iterate to put them in one folder!

Say pathsToCopy: [{ '/path/to/my/assets': '/path/to/my/output' }, '/path/to/my/other/asset'] -> Output /path/to/my/assets at /path/to/my/output and /path/to/my/other/asset at asset.

wdyt @nodejs/web-infra I want opinions

moshams272 · 2026-04-05T20:06:59Z

I'll hold off on fixing the bot's comments, Waiting for the final decision.

moshams272 added 4 commits April 5, 2026 20:04

feat: pass absolute file path from AST to metadata generator

c01a517

feat: implement asset extraction and URL rewriting in jsx-ast

01f99f0

feat: add project asset copying to web generator

c7bea21

test: add unit tests for asset extraction and URL rewriting in jsx-ast

536ed59

moshams272 requested a review from a team as a code owner April 5, 2026 19:10

vercel bot deployed to Preview April 5, 2026 19:11 View deployment

cursor bot reviewed Apr 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: assets copying#753

feat: assets copying#753
moshams272 wants to merge 4 commits intonodejs:mainfrom
moshams272:feat/ast-asset-copy

moshams272 commented Apr 5, 2026

Uh oh!

cursor bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

vercel bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

codecov bot commented Apr 5, 2026 •

edited

Loading

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Apr 5, 2026

Uh oh!

cursor bot Apr 5, 2026

Uh oh!

cursor bot Apr 5, 2026

Uh oh!

cursor bot Apr 5, 2026

Uh oh!

avivkeller commented Apr 5, 2026 •

edited

Loading

Uh oh!

moshams272 commented Apr 5, 2026 •

edited

Loading

Uh oh!

avivkeller commented Apr 5, 2026

Uh oh!

moshams272 commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

moshams272 commented Apr 5, 2026

Description

Steps:

Validation

Related Issues

Check List

Uh oh!

cursor bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

vercel bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Apr 5, 2026

Choose a reason for hiding this comment

Asset filename collisions silently overwrite different images

Uh oh!

cursor bot Apr 5, 2026

Choose a reason for hiding this comment

Use copyFile with COPYFILE_FICLONE for files

Uh oh!

cursor bot Apr 5, 2026

Choose a reason for hiding this comment

Use URL.parse() instead of new URL()

Uh oh!

cursor bot Apr 5, 2026

Choose a reason for hiding this comment

Hardcoded 'assets' path duplicated across two generators

Uh oh!

avivkeller commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moshams272 commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avivkeller commented Apr 5, 2026

Uh oh!

moshams272 commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cursor bot commented Apr 5, 2026 •

edited

Loading

vercel bot commented Apr 5, 2026 •

edited

Loading

codecov bot commented Apr 5, 2026 •

edited

Loading

Use `copyFile` with `COPYFILE_FICLONE` for files

Use `URL.parse()` instead of `new URL()`

Hardcoded `'assets'` path duplicated across two generators

avivkeller commented Apr 5, 2026 •

edited

Loading

moshams272 commented Apr 5, 2026 •

edited

Loading