fix: add parent node_modules directories #136
Conversation
| options.nodeModulesDirs.push(resolve(options.rootDir, 'node_modules')) | ||
| // Scan and add parent node_modules directory | ||
| let currentDir = resolve(options.rootDir) | ||
| while (currentDir && currentDir !== dirname(currentDir)) { |
There was a problem hiding this comment.
According to the Node.js search algorithm, this should normally happen when all parent node_modules are scanned. (https://nodejs.org/api/modules.html#modules_all_together see NODE_MODULES_PATHS section). Might this be an issue with rollup plugin?
There was a problem hiding this comment.
Yes, this PR is trying to mimic the behavior of Node. The bug of https://github.com/nuxt/framework/issues/4217 is that the dependencies are hoisted to the upper level so the rollup failed to find it. To me, it sounds reasonable for rollup to only scan the node_modules when you provide them explicitly. I think this is quite an important bug, maybe we could have it for now and raise the discussion to rollup later?
There was a problem hiding this comment.
I'm not confident to add this and masking issue without a minimal reproduction. We use plugin node-resolve that internally uses resolve package. They are supposed to do exactly this!
There was a problem hiding this comment.
Trying to reproduce. Can you please check this if can find a way to reproduce? https://github.com/pi0/nitro-reproduction
There was a problem hiding this comment.
https://github.com/antfu/nuxt-monorepo-repro
cd nuxt-app and npm run build
We had the same issue in the modules.nuxtjs.org migration here
https://vercel.com/nuxt-js/modules/5TEN9v28ppMEzdtFz5whmsnRKKcc
and workaround as
nuxt/modules@e2ac251 (#357)
With the suggestions https://github.com/nuxt/framework/issues/4217#issuecomment-1094852657 by @danielroe
There was a problem hiding this comment.
Thanks for repro but my a minimal reproduction for nitro, I meant a nitro project. I've updated mine with yarn 3 monorepo however this issue doesn't seems to exist. It is something with nuxt integration or vue alias.
There was a problem hiding this comment.
Updated your repro. By cd app && yarn build
There was a problem hiding this comment.
Thanks! Now i can reproduce it. (More precisely with externals: { inline: ['lodash-es'] })
|
Investigating more, currently, resolve package (used by rollup plugin) processes a strange search path:
Adding current workaround on top of it, fixes the issue, but also produces even more duplicates:
Another finding: Not providing
Concat
|
|
I came up with a simpler solution by adding |
🔗 Linked issue
Partially fix nuxt/nuxt#13700
It will be fixed by the combination of this PR and nuxt/framework#4488
❓ Type of change
📚 Description
📝 Checklist