Skip to content
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

fix(module): regression add module not updating nuxt.config #717

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BobbieGoede
Copy link
Member

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Had a feeling this was not (fully) working, and was reminded to check it out when the functionality was mentioned during the ecosystem meeting πŸ˜…

Fixes a regression possibly introduced in #208, it seems like it has gone unreported all this time:

If a module had to be installed as dependency, it would exit before updating the nuxt.config. Since we're checking if addDependency returns true while it's a void function (see https://github.com/nuxt/cli/blob/main/packages/nuxi/src/commands/module/add.ts#L148).

Not sure why we're not using try...catch blocks, this fix simply adds a .then(() => true) in the interest of keeping changes small/consistent (but also because I'm not sure how to trigger/test the catch case 🀭).

@BobbieGoede BobbieGoede self-assigned this Jan 31, 2025
@BobbieGoede BobbieGoede changed the title fix(module): regression add module does not update modules array fix(module): regression add module not updating nuxt.config Jan 31, 2025
@BobbieGoede BobbieGoede force-pushed the fix/add-module-update-config branch from 463c8eb to 5b74d4c Compare January 31, 2025 10:55
Copy link

pkg-pr-new bot commented Jan 31, 2025

Open in Stackblitz

npm i https://pkg.pr.new/create-nuxt-app@717
npm i https://pkg.pr.new/nuxi@717
npm i https://pkg.pr.new/@nuxt/cli@717

commit: 5b74d4c

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests βœ…

Please upload report for BASE (main@ee87c27). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #717   +/-   ##
=======================================
  Coverage        ?   13.54%           
=======================================
  Files           ?       66           
  Lines           ?     3351           
  Branches        ?       96           
=======================================
  Hits            ?      454           
  Misses          ?     2866           
  Partials        ?       31           

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants