feat(assets): resolve relative paths to co-located assets#3809
feat(assets): resolve relative paths to co-located assets#3809DraftProducts wants to merge 5 commits into
Conversation
|
@DraftProducts is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
DraftProducts
force-pushed
the
feat/relative-assets
branch
from
78283a0 to
7b0fb8d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds a co-located asset pipeline to Nuxt Content. A new Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
playground/components/content/ContentGallery.vue (1)
10-11: ⚡ Quick winPrefer unique image URL as v-for key instead of array index.
Using array index as a key can cause issues if the
itemsarray is reordered or filtered in the future. Even for a static gallery, using a unique identifier (like the image URL) is more stable and follows Vue best practices.♻️ Proposed fix to use URL-based key
<img v-for="(src, index) in items" - :key="index" + :key="src" :src="src" :alt="`Gallery image ${index + 1}`" style="max-width: 160px; height: auto; border-radius: 6px;" >This ensures each image has a stable key tied to its identity (the URL), not its position in the array.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playground/components/content/ContentGallery.vue` around lines 10 - 11, In the ContentGallery.vue v-for loop iterating over items with destructured variables (src, index), the :key attribute is currently using the array index which is unstable if the items array changes. Replace the :key="index" with :key="src" to use the unique image URL as the key instead, which provides stable identity for each image element regardless of array reordering and follows Vue best practices for list rendering.test/unit/assets.ignore.test.ts (1)
24-39: ⚡ Quick winRestore global asset extension state in teardown, not inline in a test.
setAssetExtensions([])mutates global state; restoring it inside the test body can be skipped on early failure and leak into other suites. Move restore logic toafterEach/afterAllfor deterministic isolation.Suggested patch
import { describe, it, expect, beforeAll, afterAll } from 'vitest' @@ describe('assets exclusion from content collection keys', () => { @@ afterAll(async () => { + setAssetExtensions(DEFAULT_ASSET_EXTENSIONS) await rm(dir, { recursive: true, force: true }) }) @@ it('does not filter anything when the feature is disabled', async () => { setAssetExtensions([]) @@ const keys = await source.getKeys!() expect(keys.sort()).toEqual(['clip.mp4', 'data.json', 'photo.jpg', 'post.md']) - setAssetExtensions(DEFAULT_ASSET_EXTENSIONS) }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/assets.ignore.test.ts` around lines 24 - 39, The global state restoration in the second test "does not filter anything when the feature is disabled" is being done inline at the end of the test body, which can be skipped if the test fails early and leak corrupted state into other test suites. Move the setAssetExtensions(DEFAULT_ASSET_EXTENSIONS) restoration call from the end of the test body to an afterEach hook to ensure deterministic cleanup regardless of test outcome. The initial setup of setAssetExtensions([]) in the test can remain, but the restoration must happen in a teardown hook rather than inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/plugins/websocket.dev.ts`:
- Around line 53-61: Remove the unnecessary conditional checks in the onLoad
event handler function that prevent dimension updates from being applied
unconditionally. Remove the condition from the setAttribute calls for width and
height (the check for element.width && element.height) and remove the condition
from the aspectRatio style assignment (the check for element.style.aspectRatio),
so that the incoming width and height values from the HMR payload are always
applied to set both the element attributes and the aspect ratio style whenever
width and height are provided.
- Around line 42-45: The src escaping in the websocket.dev.ts file uses a manual
regex pattern that only handles quotes and backslashes, but CSS attribute
selectors require escaping all CSS-significant characters including dots,
hashes, parentheses, brackets, braces, and spaces which are common in file paths
and asset names. Replace the manual escaping with the built-in CSS.escape
utility method on the src variable to ensure comprehensive and
standards-compliant escaping of all CSS-significant characters before using it
in the querySelectorAll attribute selector.
In `@src/utils/assets/state.ts`:
- Around line 4-9: The setAssetExtensions and getAssetExtensions functions
expose the assetExtensions array by reference, allowing external callers to
mutate it and affect global state. Modify setAssetExtensions to store a copy of
the input extensions array instead of the reference itself, and modify
getAssetExtensions to return a copy of the assetExtensions array instead of the
reference. This prevents external code from directly mutating the internal
state.
In `@test/fixtures/assets/server/api/content/get.get.ts`:
- Around line 4-6: The path variable obtained from getQuery(event).path on line
4 can be either a string or a string array when query parameters are repeated,
causing the subsequent .path() method call on line 6 to receive ambiguous input.
Normalize the path to always be a single string by extracting the first element
if it's an array, or using the original value if it's already a string, before
passing it to the .path() method call. Ensure the normalization happens between
assigning the path variable and using it in the queryCollection chain.
---
Nitpick comments:
In `@playground/components/content/ContentGallery.vue`:
- Around line 10-11: In the ContentGallery.vue v-for loop iterating over items
with destructured variables (src, index), the :key attribute is currently using
the array index which is unstable if the items array changes. Replace the
:key="index" with :key="src" to use the unique image URL as the key instead,
which provides stable identity for each image element regardless of array
reordering and follows Vue best practices for list rendering.
In `@test/unit/assets.ignore.test.ts`:
- Around line 24-39: The global state restoration in the second test "does not
filter anything when the feature is disabled" is being done inline at the end of
the test body, which can be skipped if the test fails early and leak corrupted
state into other test suites. Move the
setAssetExtensions(DEFAULT_ASSET_EXTENSIONS) restoration call from the end of
the test body to an afterEach hook to ensure deterministic cleanup regardless of
test outcome. The initial setup of setAssetExtensions([]) in the test can
remain, but the restoration must happen in a teardown hook rather than inline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d97e1cc-a842-4f2b-a781-c632bb1c7c69
⛔ Files ignored due to path filters (16)
examples/basic/content/cover.svgis excluded by!**/*.svgexamples/blog/content/blog/damavand.jpgis excluded by!**/*.jpgexamples/blog/content/blog/everest.jpgis excluded by!**/*.jpgexamples/i18n/content/en/banner.svgis excluded by!**/*.svgexamples/i18n/content/fa/banner.svgis excluded by!**/*.svgexamples/i18n/content/fr/banner.svgis excluded by!**/*.svgexamples/ui/content/cover.svgis excluded by!**/*.svgplayground/content/1.real-content/files/sample.pdfis excluded by!**/*.pdfplayground/content/1.real-content/italian-bean-stew.jpgis excluded by!**/*.jpgplayground/content/1.real-content/media/pesto-salmon-lentils.jpgis excluded by!**/*.jpgplayground/content/1.real-content/media/sample.mp4is excluded by!**/*.mp4playground/content/1.real-content/media/turkey-casserole.jpgis excluded by!**/*.jpgplayground/content/shared/sicilian-fish-stew.jpgis excluded by!**/*.jpgpnpm-lock.yamlis excluded by!**/pnpm-lock.yamltest/fixtures/assets/content/files/doc.pdfis excluded by!**/*.pdftest/fixtures/assets/content/media/photo.pngis excluded by!**/*.png
📒 Files selected for processing (43)
docs/content/docs/1.getting-started/3.configuration.mddocs/content/docs/3.files/1.markdown.mddocs/content/docs/3.files/5.assets.mdexamples/basic/content/index.mdexamples/blog/app/pages/index.vueexamples/blog/content.config.tsexamples/blog/content/blog/mount-damavand.mdexamples/blog/content/blog/mount-everest.mdexamples/i18n/content/en/index.mdexamples/i18n/content/fa/index.mdexamples/i18n/content/fr/index.mdexamples/ui/content/index.mdpackage.jsonplayground/components/content/ContentGallery.vueplayground/content.config.tsplayground/content/1.real-content/assets-demo.mdplayground/content/1.real-content/media/sample.htmlplayground/nuxt.config.tssrc/module.tssrc/runtime/plugins/websocket.dev.tssrc/types/module.tssrc/utils/assets/discover.tssrc/utils/assets/index.tssrc/utils/assets/paths.tssrc/utils/assets/rewrite.tssrc/utils/assets/shared.tssrc/utils/assets/state.tssrc/utils/dev.tssrc/utils/source.tstest/assets.test.tstest/fixtures/assets/app/app.vuetest/fixtures/assets/content.config.tstest/fixtures/assets/content/index.mdtest/fixtures/assets/content/media/page.htmltest/fixtures/assets/content/posts/nested.mdtest/fixtures/assets/nuxt.config.tstest/fixtures/assets/package.jsontest/fixtures/assets/server/api/content/get.get.tstest/unit/assets.ignore.test.tstest/unit/assets.parser.test.tstest/unit/assets.paths.test.tstest/unit/assets.rewrite.test.tstest/unit/devCache.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@playground/components/content/ContentGallery.vue`:
- Line 11: The `:key="src"` binding in the gallery iteration can produce
duplicate keys if the gallery array contains duplicate URLs, causing unstable
DOM updates. Replace this key binding with a guaranteed-unique composite key by
combining the src value with the array index using template string syntax like
`${src}-${index}`, or alternatively use a stable unique identifier from your
gallery data if one exists. This will ensure each item has a stable, unique key
for proper Vue rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 213396e3-c3ef-4370-b3b2-c714bc7cdab0
📒 Files selected for processing (5)
playground/components/content/ContentGallery.vuesrc/runtime/plugins/websocket.dev.tssrc/utils/assets/state.tstest/fixtures/assets/server/api/content/get.get.tstest/unit/assets.ignore.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/fixtures/assets/server/api/content/get.get.ts
- src/utils/assets/state.ts
- test/unit/assets.ignore.test.ts
- src/runtime/plugins/websocket.dev.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/module.ts`:
- Around line 235-237: The object being pushed to config.publicAssets does not
match the PublicAssetDir type required by Nitro, causing a TypeScript
compilation error. Add a type assertion to the object literal being pushed in
the config.publicAssets.push call to cast it as PublicAssetDir or the
appropriate Nitro type. If PublicAssetDir is not available, check the existing
Nitro type imports in the file to use the correct type name, or use a type
assertion to resolve the type mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 129dba16-63fc-4b65-b5d3-a87713f1928a
📒 Files selected for processing (2)
docs/content/docs/3.files/5.assets.mdsrc/module.ts
✅ Files skipped from review due to trivial changes (1)
- docs/content/docs/3.files/5.assets.md
| if (!config.publicAssets.some(asset => asset?.dir === assets.publicDir)) { | ||
| config.publicAssets.push({ dir: assets.publicDir }) | ||
| } |
There was a problem hiding this comment.
Fix TypeScript error: PublicAssetDir type mismatch.
The CI fails because { dir: string } isn't assignable to Nitro's PublicAssetDir type. Add a type assertion to satisfy the compiler.
🐛 Proposed fix
- if (!config.publicAssets.some(asset => asset?.dir === assets.publicDir)) {
- config.publicAssets.push({ dir: assets.publicDir })
+ if (!config.publicAssets.some(asset => typeof asset === 'object' && asset?.dir === assets.publicDir)) {
+ config.publicAssets.push({ dir: assets.publicDir } as import('nitro/types').PublicAssetDir)
}Alternatively, if the nitro/types import path differs in this codebase, use as never as a last resort or check the existing Nitro type imports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!config.publicAssets.some(asset => asset?.dir === assets.publicDir)) { | |
| config.publicAssets.push({ dir: assets.publicDir }) | |
| } | |
| if (!config.publicAssets.some(asset => typeof asset === 'object' && asset?.dir === assets.publicDir)) { | |
| config.publicAssets.push({ dir: assets.publicDir } as import('nitro/types').PublicAssetDir) | |
| } |
🧰 Tools
🪛 GitHub Actions: ci / 0_ubuntu.txt
[error] 236-236: TypeScript (TS2345): Argument of type '{ dir: string; }' is not assignable to parameter of type 'PublicAssetDir'.
🪛 GitHub Actions: ci / ubuntu
[error] 236-236: TypeScript (TS2345): Argument of type '{ dir: string; }' is not assignable to parameter of type 'PublicAssetDir'.
🪛 GitHub Check: ubuntu
[failure] 236-236:
Argument of type '{ dir: string; }' is not assignable to parameter of type 'PublicAssetDir'.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/module.ts` around lines 235 - 237, The object being pushed to
config.publicAssets does not match the PublicAssetDir type required by Nitro,
causing a TypeScript compilation error. Add a type assertion to the object
literal being pushed in the config.publicAssets.push call to cast it as
PublicAssetDir or the appropriate Nitro type. If PublicAssetDir is not
available, check the existing Nitro type imports in the file to use the correct
type name, or use a type assertion to resolve the type mismatch.
Source: Linters/SAST tools
|
Thanks for the PR @DraftProducts, |
|
@farnabaz thank you for your answer. |
2 participants
❓ Type of change
📚 Description
Right now you can't reference an asset that sits next to your content. An image used in a markdown file has to live in
public/and be linked with an absolute path, a relative path to a file in the same folder doesn't resolve.This PR resolves those relative paths. You can reference images, video and documents relative to your content file (in markdown, MDC, raw HTML and frontmatter), and they get copied next to the built site with the references rewritten to served URLs at build time.
Absolute paths (
/image.png) and remote URLs are left as they are.This is especially useful with v3's remote sources. A content repo can carry its own images and stay self-contained, so you can keep content and media in a separate sub-repo instead of duplicating assets into every app's
public/.A long-standing request
This predates v3. The original feature request, #2163 ("Built in support for images under content dir"), is from July 2023. Almost three years on, it's still an open gap: #3140 ("How to handle content assets?") is labelled enhancement and help wanted, and one commenter "solved" it by switching to Astro instead.
The module's own tracker tells the same story, with v3 migration requests stacking up against an architecture it can't follow without a rewrite.
This PR closes #3140 and answers the same need raised across both trackers:
Credit
This is based on Dave Stewart's
nuxt-content-assets, which did this for v2 and worked out the design: relative resolution in the body and frontmatter, image-size injection, ordering-prefix stripping, dev live-reload. Thanks to Dave for that work.In core vs a module
A few things work better merged in than bolted on:
node_modules/.../cachefolder to locate, no "register before @nuxt/content" ordering, no second config to keep in sync. It tracks Content's internals instead of breaking on them, which is the exact failure mode the standalone module keeps hitting (File scanner does not work with Content v2.13+ (Nuxt 4 directory structure) davestewart/nuxt-content-assets#85 on Content 2.13, [Nuxt 4] TypeError: Cannot read properties of undefined (reading 'ws') on dev startup davestewart/nuxt-content-assets#105 on Nuxt 4).Features
aspect-ratiois added to images by default to avoid layout shift.width/heightattributes or a?width=&height=query are options.1.,01.) are stripped from URLs the same way they are for routes.content.build.assets(imageSize,extensions,blankLinks,prefix,debug). Setenabled: falseto turn it off.📝 Checklist