Broken with yarn PNP

[mijamo]

Currently if you use Yarn PNP you get Error: [vite-plugin-mdx] "react" must be installed

This is because this package is using find-dependency which seems to be a rather confidential library that tries to get the path of a dependency only using node_modules, a trivial but often wrong approach of importing in node.

I haven't looked much into why this library is trying to resolve dependencies itself. Usually it should be left to Vite to handle this kind of things (because some Vite config options such as aliases can impact the resolution, and also because resolving import is actually a very complex task). If I get a bit of background on the why I can try to write some pull request to fix it.

[brillout]

Vite doesn't support Yarn PNP. And it doesn't look like it's going to change anytime soon.

[mijamo]

Vite does support Yarn PnP. I use it in 4 different repositories without issues even using quite complex monorepo setup with linking and common dependencies.

[brillout]

Vite's source code assumes dependencies to live in node_modules/. Are you using some kind of compatibility mode that somehow fakes modules living in node_modules/?

With Yarn PnP you mean Yarn's feature of saving dependencies in a single archive file (that you typically commit to have a plug-and-play like DX), correct?

[mijamo]

I am not aware of such limit of Vite. Vite even has internal utility function to detect yarn PnP (https://github.com/vitejs/vite/blob/06b9935208abaa7885ded2a780cbb5699d14c8da/packages/vite/src/node/utils.ts#L36) Yarn PNP is https://yarnpkg.com/features/pnp and basically what you described (although the commit part is separated and not needed for Yarn PnP). You have an archive that contains all the code, and a pnp.js file that injects the dependencies into Node on runtime so that Node finds the dependencies without trouble. Yarn PnP has an API as well that resolves paths, but as my first comment mentionned I think in any case resolving dependencies intuitively seems out of scope for this package. Even the vite-vue-plugin does not try to resolve imports for instance.

I haven't tested but I find the current setup also breaks import aliases supported by Vite, and potentially many other cases where the import is not straightforward node_modules/path.

[mijamo]

I checked a bit more and Vite uses https://github.com/browserify/resolve under the hood to resolve packages, and it supports PnP without issue and seems much more popular and maintained than find-dependency used here. So maybe if it is really relevant for this package to resolve dependency it would be a good replacement?

[brillout]

Interesting that Vite + Yarn PnP is working for you. The Vite source code has a couple of places where it assumes the file path of dependencies to contain node_modules.

Nice that I was wrong and that Yarn PnP actually works.

I checked a bit more and Vite uses https://github.com/browserify/resolve under the hood to resolve packages, and it supports PnP without issue and seems much more popular and maintained than find-dependency used here. So maybe if it is really relevant for this package to resolve dependency it would be a good replacement?

@aleclarson

[aleclarson]

@mijamo Could you put together a PnP example in the ./examples folder of this repo?

See #12 to understand why find-dependency is currently used. You could try replacing it with browserify/resolve, but make sure it doesn't break what #12 fixed.

[mijamo]

Thanks for pointing the direction. I'll try to find some time in the coming days to look at it.

[merceyz]

With Yarn PnP you mean Yarn's feature of saving dependencies in a single archive file (that you typically commit to have a plug-and-play like DX), correct?

It's a single zip archive per dependency, not a single archive with all dependencies. The commit part is https://yarnpkg.com/features/zero-installs which isn't needed for Yarn PnP, it's fully optional.

See https://yarnpkg.com/features/pnp for more details

The Vite source code has a couple of places where it assumes the file path of dependencies to contain node_modules.

The path to dependencies do contain node_modules so that's not an issue.

// Looks similar to `/path/to/.yarn/cache/lodash-npm-4.17.11-1c592398b2-8b49646c65.zip/node_modules/lodash/ceil.js`
const lodashCeilPath = require.resolve(`lodash/ceil`);

https://yarnpkg.com/features/pnp#packages-are-stored-inside-zip-archives-how-can-i-access-their-files

[brillout]

@merceyz Thanks good to know