-
Notifications
You must be signed in to change notification settings - Fork 90
feat: Yarn PnP support #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| const targetRoot = rootDir || process.cwd(); | ||
| const nodeModulesPath = join(targetRoot, 'node_modules'); | ||
| if (!existsSync(nodeModulesPath)) { | ||
| mkdirSync(nodeModulesPath, { recursive: true }); | ||
| } | ||
| cachedNodeModulesDir = nodeModulesPath; | ||
| return cachedNodeModulesDir; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using vite project's root instead of traversing for existing folder because pnp doesn't generate node_modules and this seems more natural
| const virtualDirPath = VirtualModule.getVirtualModuleDirPath(); | ||
| (config.resolve as any).alias.push({ | ||
| find: new RegExp(`^${virtualDir}/`), | ||
| replacement: `${virtualDirPath}/`, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using alias to map path to __mf__virtual
| function searchPackageVersion(sharedName: string): string | undefined { | ||
| try { | ||
| const sharedPath = require.resolve(sharedName); | ||
| const sharedPath = require.resolve(sharedName, { paths: [process.cwd()] }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to use resolved config root instead of cwd but this seems fine for now
gioboa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @ha1fstack
Unfortunately, in these days I'm quite busy and I can't review the PR as fast as I usually do.
I'm going to look at it in the next days.
closes #155
Problem
Yarn PnP can not resolve arbitary qualified resolutions (or packages) whose are not declared explicitly.
However this plugin relies on on-the-fly generated __mf__virtual package inside physical node_modules folder - so it was not compatible
Tries
I first tried to use Vite's virtual module (in-memory) instead of real fs. But it did not work well since this plugin seems to be relying on (the intend is not documented so this is a guess) Vite's dependency pre-bundling - but virtual modules cannot be pre-bundled.
Solution
The root cause was not fs, so I focused on the qualified / unqualified resolution part. Instead of making the generated code as a package (with package.json and importing it with name), using unqualified resolution (real path) + creating alias to that path to keep things simple solved the issue.
Also fixed the problematic searchPackageVersion util too since the packages needs to be resolved from the host package (the service) root not the vite-mf's package root.