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(upgrade): warmup nypm before removing node_modules #688

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

This is a silly hack/fix for #675, it works by calling removeDependency to trigger a dynamic import in nypm before we remove node_modules. Without this change the dynamic import would be triggered by addDependency which is after removing node_modules which means it will resolve to undefined and error/exit.

@danielroe
This works but relies on internal behavior of nypm which could change/break, I'm not sure what a clean/proper way would be to fix this 😅

Resolves #675

@BobbieGoede BobbieGoede self-assigned this Jan 18, 2025
Copy link

pkg-pr-new bot commented Jan 18, 2025

Open in Stackblitz

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

commit: def5fc9

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

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

Files with missing lines Patch % Lines
packages/nuxi/src/commands/upgrade.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##             main    #688   +/-   ##
======================================
  Coverage        ?   3.31%           
======================================
  Files           ?      65           
  Lines           ?    3133           
  Branches        ?      49           
======================================
  Hits            ?     104           
  Misses          ?    2994           
  Partials        ?      35           

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

@danielroe
Copy link
Member

@pi0 do you have any better idea?

currently the dynamic import in nypm produces a separate chunk that triggers this issue, even when inlining all deps

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.

nuxi upgrade does not work any longer: Cannot find module
3 participants