fix: get mdc configs by calling mdc:configSources hook#3736
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoved the exported Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
danielroe
force-pushed
the
fix/mdc-config-order
branch
from
8547381 to
a426486
Compare
danielroe
changed the title
nuxt.ready rather than in modules:donemdc:configSources hook
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/mdc.ts (1)
19-30: Prevent duplicate config resolution on concurrent first access.Lines 21-27 can run more than once if multiple calls happen before
_configsis set. Cache the in-flight promise too.♻️ Suggested change
let _configs: MdcConfig[] | undefined +let _configsPromise: Promise<MdcConfig[]> | undefined setMdcConfigResolver(async () => { - if (!_configs) { - const paths: string[] = [] - await nuxt.callHook('mdc:configSources', paths) - const jiti = createJiti(nuxt.options.rootDir) - _configs = paths.length - ? await Promise.all(paths.map(path => jiti.import(path).then(m => (m as { default: MdcConfig }).default || m))) - : [] - } - return _configs + if (_configs) { + return _configs + } + _configsPromise ||= (async () => { + const paths: string[] = [] + await nuxt.callHook('mdc:configSources', paths) + const jiti = createJiti(nuxt.options.rootDir) + _configs = paths.length + ? await Promise.all(paths.map(path => jiti.import(path).then(m => (m as { default: MdcConfig }).default || m))) + : [] + return _configs + })() + return _configsPromise })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/mdc.ts` around lines 19 - 30, The resolver passed to setMdcConfigResolver can run multiple times concurrently because only the resolved _configs is cached; instead cache the in-flight promise so concurrent calls await the same work. Modify the resolver in src/utils/mdc.ts to use a module-level variable (e.g., _configsPromise) and set it when starting the async work (calling nuxt.callHook('mdc:configSources'), createJiti(...) and Promise.all(paths.map(... jiti.import ...))) and have subsequent calls return await _configsPromise; once fulfilled assign the resulting array to _configs and clear/retain the promise as appropriate so only the first invocation does the import work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/mdc.ts`:
- Around line 23-26: The call to nuxt.callHook('mdc:configSources', paths) uses
an unreleased feature (PR `#471`) so paths may remain empty with
`@nuxtjs/mdc`@0.20.1; update the call to be defensive: detect the installed mdc
version or the presence of the hook support and only call
nuxt.callHook('mdc:configSources', paths) when supported, otherwise fall back to
the existing default sources or explicitly populate paths; reference the symbols
nuxt.callHook, 'mdc:configSources', and the paths variable in src/utils/mdc.ts,
or alternatively update the dependency constraint to a version that includes PR
`#471` before relying on the hook.
---
Nitpick comments:
In `@src/utils/mdc.ts`:
- Around line 19-30: The resolver passed to setMdcConfigResolver can run
multiple times concurrently because only the resolved _configs is cached;
instead cache the in-flight promise so concurrent calls await the same work.
Modify the resolver in src/utils/mdc.ts to use a module-level variable (e.g.,
_configsPromise) and set it when starting the async work (calling
nuxt.callHook('mdc:configSources'), createJiti(...) and
Promise.all(paths.map(... jiti.import ...))) and have subsequent calls return
await _configsPromise; once fulfilled assign the resulting array to _configs and
clear/retain the promise as appropriate so only the first invocation does the
import work.
farnabaz
approved these changes
Mar 26, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🔗 Linked issue
❓ Type of change
📚 Description
discovered whilst working on nuxt-content-twoslash, currently if a module also hooks into
mdc:configSourcesbut does so after nuxt content, any changes it makes will be silently dropped due to ordering.this instead moves to call the hook (and depends on nuxt-content/mdc#471)
Important
this should not be released before nuxt-content/mdc#471. the mdc pr is safe to release on its own, but it should be released, and then this PR should be released, depending on the newer version of mdc, or we'll lose 'local'
mdc.config.tsfiles📝 Checklist