fix(externals): resolve relative imports when checking for externals

[danielroe]

🔗 Linked issue

resolves #386, https://github.com/nuxt/framework/issues/5348

❓ Type of change

• [ ] 📖 Documentation (updates to the documentation or readme)
• [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
• [ ] 👌 Enhancement (improving an existing functionality like performance)
• [ ] ✨ New feature (a non-breaking change that adds functionality)
• [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Previously relative imports from modules were skipped/inlined, resulting in this behaviour:

// app.vue
import { Pagination } from "swiper";
// swiper is an external, but this becomes, in `server.mjs`:
import Pagination from './modules/pagination/pagination.js';
// with this PR:
import Pagination from 'file:///my/project/node_modules/.pnpm/[email protected]/node_modules/swiper/modules/pagination/pagination.js';

📝 Checklist

• [x] I have linked an issue or discussion.
• [ ] I have updated the documentation accordingly.

[codecov[bot]]

Codecov Report

Merging #387 (94b0845) into main (2a82973) will decrease coverage by 0.30%. The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             main     nuxt/framework#387      +/-   ##
==========================================
- Coverage   63.16%   62.85%   -0.31%     
==========================================
  Files          54       54              
  Lines        3619     3624       +5     
  Branches      382      382              
==========================================
- Hits         2286     2278       -8     
- Misses       1321     1333      +12     
- Partials       12       13       +1     

Impacted Files	Coverage Δ
src/rollup/plugins/externals.ts	79.28% <87.50%> (-4.87%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[pi0]

@danielroe Do you have any update on this? 🙏🏼 Need to finalize state of this PR :)

[danielroe]

Updated in linked issue 👍

[pi0]

Checking back on this issue, interestingly it is only reproducible on external repo + using custom srcDir + externals.trace=false (enabled for prerender). https://stackblitz.com/edit/github-y4xqiy-q9qfjg?file=nitro.config.ts

Changes by your PR fixes the issue and generate import with right cde but I'm afraid we are missing some other root causes. One of them is that swiper is detected by mlly as invalid esm import (hence auto inlined) while it is valid.

[danielroe]

Nice finds. Would be very happy to look into those issues as well 👍 Do you think that means we shouldn't merge this PR, or that we should both merge it and resolve them?

[pi0]

I would suggest first fixing the root causes and see if we can make a fix limited to trace: false (which is actually causing problem) instead of doing modification to originalId.

[pi0]

Fix for swiper wrong detection: https://github.com/unjs/mlly/commit/8e866c3459c04163bb59dd56469f8468add899b3

[danielroe]

I still think this is a safe and appropriate change to make but can't reproduce the linked issues. I'm happy if you want to close it; we can always come back to it later if needed.

[pi0]

Thanks for confirming! I am always up to revising this when needed (which i hopefully by following up the inline-first approach will be even less likely!