Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(vite): respect ctx.nuxt.options.modulesDir for resolving externals with vite-node #7612

Merged
merged 1 commit into from Sep 16, 2022
Merged

fix(vite): respect ctx.nuxt.options.modulesDir for resolving externals with vite-node #7612

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vite/src/vite-node.ts
View file
Open in desktop
Expand Up @@ -102,7 +102,7 @@ function createViteNodeMiddleware (ctx: ViteBuildContext, invalidates: Set<strin
node.shouldExternalize = async (id: string) => {
const result = await isExternal(id)
if (result?.external) {
return resolveModule(result.id, { url: ctx.nuxt.options.rootDir })
return resolveModule(result.id, { url: ctx.nuxt.options.modulesDir })
Copy link
Member Author

@pi0 pi0 Sep 16, 2022 •
edited

Choose a reason for hiding this comment

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

@antfu Shall we do this to be same as vite-node? Would it diretly require vue or let vite resolve to fullpath?

Suggested change
return resolveModule(result.id, { url: ctx.nuxt.options.modulesDir })
return id

Copy link
Member

Choose a reason for hiding this comment

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

The id passed here should already be resolved by Vite. So yes I think we could directly do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for checking. FYI, it is not always resolved. You can reproduce it only with an external repository by putting a debugger. vue for instance is directly passed to shouldExternalize.

}
return false
}
Expand Down