ensure we don't resolve modules in parent folders

[rchl]

I've stumbled upon a crash trying to start a Nuxt 3-based project in a subdirectory of a Nuxt 2 project.

So the main Nuxt 2 project at /project/ and a Nuxt 3-based docs subdirectory at /project/docs. This crashes due to nuxt dependency from the root project getting picked up.

It seems the issue is in this code:

https://github.com/unjs/mlly/blob/6327ad8d42ba1fb1672fbc27283d7dca5a714699/src/resolve.ts#L75-L79

This _resolve function receives an id parameter with a nuxt value. It creates url with the value of the current working directory so /project/docs. Then new URL("./", url) actually resolves to the /project/ directory instead of /project/docs which seems very much not expected to me.

Try this in a browser:

new URL('./', 'file:///project/docs').href
"file:///project/"

(Same for the new URL("node_modules", url) line above which seems to be scheduled for removal at some point)

[rchl]

BTW. The chain of calls that leads to this code is:

nuxi (loadKit) -> @nuxt/kit (loadNuxt) -> pkg-types (resolvePackageJSON) -> mlly (resolvePath) -> mlly (_resolve)

[rchl]

Hmm, there is more issues like that elsewhere. For example in getPackageScopeConfig which I think comes from internal Node libs. So I think the solution to issues like that is ensure that the base URL ends in slash if it's a directory.

new URL('./', 'file:///project/docs/').href
"file:///project/docs/"

[rchl]

This rabbit hole goes deep. The first directory that is involved and that doesn't include the trailing slash is the rootDir one resolved in https://github.com/nuxt/nuxt/blob/184d57bb1923b8bf21a372eabdb67e1241d2023f/packages/nuxi/src/commands/dev.ts#L41. Forcing trailing slash here fixes this specific issue but creates another. So it's no longer clear to me what and where should be fixed. It seems to me like using URI for resolving directory paths is a little brittle since it's not clear whether path is to a directory or a file without doing file IO.

[pi0]

Hi dear @rchl. Thanks for sharing all investigations.

Yes, that's kinda tricky situation. We removed the directory URL workaround but it introduced other regressions in Nuxt 3.

I think first good step would be making a minimal mlly reproduction to test the behavior if you can help 🙏🏼 . We might try to introduce a major version on progressively adopt it within nuxt ecosystem.

[rchl]

I think first good step would be making a minimal mlly reproduction to test the behavior if you can help 🙏🏼

I'm thinking that a unittest for that would be a good reproduction? So something like https://github.com/unjs/mlly/blob/main/test/fixture/resolve.mjs but I guess this is just a lone file that is not really testing anything?

[pi0]

Yeah. We probably could make an external repo for it. Especially trying node_modules resolution too via an exclude in .gitignore and then moving it to fixtures (because previous regression happened this way).

[cjpearson]

~~It seems there may also be an upstream issue here. When I patch mlly to use import.meta.resolve instead of import-meta-resolve, the issue is fixed.~~

~~I also see a difference with a quick test script~~

import { pathToFileURL } from 'url'
import { resolve } from 'import-meta-resolve'

const base = pathToFileURL(process.cwd())

console.log(import.meta.resolve('nuxt', base)) // returns ./node_modules/nuxt
console.log(resolve('nuxt', base)) // returns ../../node_modules/nuxt

Never mind, this was incorrect.

[cjpearson]

So I looked into this one a bit more. Originally I thought it may be an issue with import-meta-resolve, but the semantics there are the same as node's built-in import.meta.resolve.

While it is a bit unintuitive, it probably makes sense for mlly to keep the same behavior. For the downstream issue, I've filed a separate fix. Here Nuxt knows the path should represent a directory and it should be safer to automatically add the trailing slash.

[pi0]

Yes, or better, always provide a URL as parent which is compatible with ESM resolution. (dir in URL does not makes sense)