Skip to content

refactor(kit,nuxi): resolve module paths using node algorithm#19537

Merged
danielroe merged 8 commits into
mainfrom
fix/esm-utils
Mar 10, 2023
Merged

refactor(kit,nuxi): resolve module paths using node algorithm#19537
danielroe merged 8 commits into
mainfrom
fix/esm-utils

Conversation

@danielroe

@danielroe danielroe commented Mar 8, 2023

Copy link
Copy Markdown
Member

🔗 Linked issue

#18431 (comment)

❓ 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 like performance)
  • ✨ 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

The linked issue is caused by us trying to resolve an ESM module's path by CJS utilities, and then import it with native ESM.

This refactors our internal CJS utils in kit + nuxi to use resolvePath from mlly which uses import-meta-resolve under the hood (a polyfill for the Node built-in). To decrease the change surface, I've restricted changes to places where we have a single search path (e.g. rootDir or cwd) and have kept the existing behaviour where we have multiple paths (e.g. where nuxt.options.modulesDir is used).

I would be up for refactoring the remaining usage of CJS utils as well, but am also happy to defer that to a subsequent PR.

As part of this, the behaviour of tryResolveModule (exposed by kit) has changed (it is now async, it now returns a file URL, and it takes a single string as a second argument rather than an object). However, I think this is safe as it was a deprecated internal module and not used outside of nuxt itself.

I've also decreased the number of internal utilities we are exposing from the CJS part of kit as a number of them are not being used elsewhere in the repo.

📝 Checklist

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

@danielroe danielroe added 3.x 🐛 bug Something isn't working as expected kit Nuxt Kit module utilities labels Mar 8, 2023
@danielroe danielroe requested a review from pi0 March 8, 2023 20:54
@danielroe danielroe self-assigned this Mar 8, 2023
@codesandbox

codesandbox Bot commented Mar 8, 2023

Copy link
Copy Markdown

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@danielroe danielroe requested a review from antfu March 9, 2023 15:09
@pi0

pi0 commented Mar 10, 2023

Copy link
Copy Markdown
Member

This is a really lovely initiative! Only I think we should use mlly.resolvePath that wraps same package internally for less deps, more consistency and path normalizations.

@danielroe danielroe changed the title refactor(kit,nuxi): use import-meta-resolve to resolve module paths refactor(kit,nuxi): resolve module paths using node resolver algorithm Mar 10, 2023

@pi0 pi0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good must have to me 🚀

@danielroe danielroe changed the title refactor(kit,nuxi): resolve module paths using node resolver algorithm refactor(kit,nuxi): resolve module paths using node algorithm Mar 10, 2023
@danielroe danielroe merged commit 6d79b71 into main Mar 10, 2023
@danielroe danielroe deleted the fix/esm-utils branch March 10, 2023 14:55
@danielroe danielroe mentioned this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x 🐛 bug Something isn't working as expected kit Nuxt Kit module utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants