feat(module)!: use moduleDependencies to manipulate options#5384
feat(module)!: use moduleDependencies to manipulate options#5384benjamincanac merged 12 commits intov4from
moduleDependencies to manipulate options#5384Conversation
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
moduleDependencies to manipulate options
commit: |
|
@danielroe I'm a bit hesitant to merge this as it will break apps that haven't been updated to https://github.com/nuxt/content/releases/tag/v3.8.1. We would need to update the |
|
once I merge nuxt/nuxt#33689, we should be fine with the latest nuxt patch even if uses don't update content |
|
@danielroe Should I merge this for the next minor? |
📝 WalkthroughWalkthroughThe module refactoring shifts dependency management from inline registration ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/module.ts (2)
239-248: Conditional logic may prevent fallback stub from being applied.Since
colorModedefaults totrue(per JSDoc at line 31), the conditionoptions.colorMode || hasNuxtModule('@nuxtjs/color-mode')will almost always be truthy, preventing the else branch from adding the stub composable.If the intent is to add color-mode components only when the module is actually present (and fallback to stub otherwise), consider changing the logic to rely solely on module presence:
- if (options.colorMode || hasNuxtModule('@nuxtjs/color-mode')) { + if (hasNuxtModule('@nuxtjs/color-mode')) {Otherwise, if
moduleDependenciesguarantees the module is installed whencolorMode !== false, this works correctly, but theoptions.colorModecheck becomes redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module.ts` around lines 239 - 248, The current conditional uses "options.colorMode || hasNuxtModule('@nuxtjs/color-mode')" which makes the truthy default options.colorMode override module detection and skip the fallback stub; change the logic to base the branch on the actual module presence only (i.e., use hasNuxtModule('@nuxtjs/color-mode') to decide when to call addComponentsDir for './runtime/components/color-mode' and otherwise call addImportsDir('./runtime/composables/color-mode'), or conversely make the check explicitly test options.colorMode !== false && hasNuxtModule(...)) so that addComponentsDir and addImportsDir are invoked correctly; update references to options.colorMode, hasNuxtModule, addComponentsDir, and addImportsDir in module.ts accordingly.
145-152: Mark@nuxtjs/color-modeasoptional: trueinmoduleDependenciesto be more defensive and consistent with the@nuxtjs/mdcpattern.Currently,
@nuxtjs/color-modeis conditionally added tomoduleDependencieswithout the optional flag. SincecolorModedefaults totrue, this declares the module as required by default. However, the stub composable at line 247 only handles the case wherecolorModeis explicitly disabled, not when the module is missing from an installation. Addingoptional: truewould match the defensive pattern used for@nuxtjs/mdc(line 154) and ensure the module gracefully falls back to stubs when unavailable, regardless of the default setting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module.ts` around lines 145 - 152, Update the entry for '@nuxtjs/color-mode' in moduleDependencies to mark it optional (similar to the '@nuxtjs/mdc' pattern) so the module system won't treat it as required; locate where moduleDependencies is populated (the conditional using userUiOptions.colorMode and the object for '@nuxtjs/color-mode') and add optional: true to that object so the stub composable fallback works whether the module is disabled or simply not installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/module.ts`:
- Around line 239-248: The current conditional uses "options.colorMode ||
hasNuxtModule('@nuxtjs/color-mode')" which makes the truthy default
options.colorMode override module detection and skip the fallback stub; change
the logic to base the branch on the actual module presence only (i.e., use
hasNuxtModule('@nuxtjs/color-mode') to decide when to call addComponentsDir for
'./runtime/components/color-mode' and otherwise call
addImportsDir('./runtime/composables/color-mode'), or conversely make the check
explicitly test options.colorMode !== false && hasNuxtModule(...)) so that
addComponentsDir and addImportsDir are invoked correctly; update references to
options.colorMode, hasNuxtModule, addComponentsDir, and addImportsDir in
module.ts accordingly.
- Around line 145-152: Update the entry for '@nuxtjs/color-mode' in
moduleDependencies to mark it optional (similar to the '@nuxtjs/mdc' pattern) so
the module system won't treat it as required; locate where moduleDependencies is
populated (the conditional using userUiOptions.colorMode and the object for
'@nuxtjs/color-mode') and add optional: true to that object so the stub
composable fallback works whether the module is disabled or simply not
installed.
|
yes, it should be 🙏 |
|
How does this work? Do I need to configure it? :) |
🔗 Linked issue
❓ Type of change
📚 Description
this uses the new module dependencies feature in https://github.com/nuxt/nuxt/releases/tag/v4.1.0 to specify options for other modules (and require them).
remake of #4927, pending a fix in nuxt/content
📝 Checklist