Skip to content
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

Support Yarn PnP #77

Open
ezracelli opened this issue Jun 30, 2023 · 6 comments
Open

Support Yarn PnP #77

ezracelli opened this issue Jun 30, 2023 · 6 comments

Comments

@ezracelli
Copy link

PnP (a module installation strategy used by Yarn Berry) stores modules in .zip files. When a package is imported, the package manager creates an isolated, read-only virtual filesystem. More info: https://yarnpkg.com/features/pnp

telefunc/vite#plugin has a side-effect that attempts to write to one of telefunc's own files. Due to the read-only nature of the virtual filesystem created by PnP, this write fails.

function plugin(config?: ConfigUser): any {
importGlobOn()

function importGlobOn() {
writeFileSync(

Workaround

I patched the module manually.
diff --git a/dist/cjs/node/vite/importGlob/telefuncFilesGlob.js b/dist/cjs/node/vite/importGlob/telefuncFilesGlob.js
index 12d2beb0902533d560a37205d5c1779a41192d46..a9369b2a79929fd8dc53f02a50bc5d6cf000ae84 100644
--- a/dist/cjs/node/vite/importGlob/telefuncFilesGlob.js
+++ b/dist/cjs/node/vite/importGlob/telefuncFilesGlob.js
@@ -1,4 +1 @@
-"use strict";
-Object.defineProperty(exports, "__esModule", { value: true });
-exports.importGlobUnset = void 0;
-exports.importGlobUnset = true;
+export const telefuncFilesGlob = import.meta.glob("/**/*.telefunc.(js|cjs|mjs|ts|cts|mts|jsx|cjsx|mjsx|tsx|ctsx|mtsx)");
diff --git a/dist/cjs/node/vite/index.js b/dist/cjs/node/vite/index.js
index dd988d6ee4dd2dd361d5461df11d9742ab18b940..83523a7ccfadf5486f2899699dcd4ac3ba55c1e3 100644
--- a/dist/cjs/node/vite/index.js
+++ b/dist/cjs/node/vite/index.js
@@ -12,10 +12,8 @@ const importBuild_1 = require("./plugins/importBuild");
 const previewConfig_1 = require("./plugins/previewConfig");
 const printShieldGenResult_1 = require("./plugins/printShieldGenResult");
 const manifest_1 = require("./plugins/manifest");
-const toggle_1 = require("./importGlob/toggle");
 // Return as `any` to avoid Plugin type mismatches when there are multiple Vite versions installed
 function plugin(config) {
-    (0, toggle_1.importGlobOn)();
     const plugins = [
         (0, transform_1.transform)(),
         (0, commonConfig_1.commonConfig)(),
@brillout brillout changed the title Vite plugin is incompatible with PnP Support Yarn PnP Jul 1, 2023
@brillout brillout added the enhancement ✨ New feature or request label Jul 1, 2023
@brillout
Copy link
Owner

brillout commented Jul 1, 2023

Indeed. FYI the side-effect will be removed soon. In the meantime use pnpm (recommended) or npm instead.

@rtritto
Copy link
Contributor

rtritto commented Jan 17, 2025

@ezracelli @dvteixeira24 is the fix (workaround) also compatible with npm and pnpm?

@rtritto
Copy link
Contributor

rtritto commented Jan 17, 2025

Indeed. FYI the side-effect will be removed soon. In the meantime use pnpm (recommended) or npm instead.

@brillout also pnpm can use PnP with node-linker=pnp (https://pnpm.io/npmrc#node-linker).

Can I help with a PR to remove the side-effects? If yes, there are some guidelines?

@brillout
Copy link
Owner

will be removed soon

I got sidetracked by Vike's success. It's still very much on the radar, but it requires some complex restructuring (breaking down the Telefunc package into multiple packages @telefunc/vite, @telefunc/next, ....). I'll work on it after Vike hits 1.0 (ETA Q1).

Can I help with a PR to remove the side-effects?

That'd be great. Although, for this one, it's better I do it. Would you be up to contribute on something else?

@brillout
Copy link
Owner

Can I help with a PR to remove the side-effects?

Actually, we can give it a try. Let's see how it goes 👀 But let's be open to finish the PR later if it get's too complex.

If yes, there are some guidelines?

Yes, have a look at Telefunc's Vite plugin, you'll see the code that writes to node_modules/. You can try to remove that code while making the CI happy. Let's see how far we can get without major restructuring.

Would you be up to contribute on something else?

See vikejs/vike#1349, most notably:

Looking forward to it!

@rtritto
Copy link
Contributor

rtritto commented Jan 18, 2025

Actually, we can give it a try. Let's see how it goes 👀 But let's be open to finish the PR later if it get's too complex.

Since Telefunc will be splitted in multiple packages, it doesn't seem to make sense to start a PR now.
Actually, for this issue, I can't use Telefunc in my projects (all PnP based), but I can wait and work on other things.


Thanks for the recap of the activities, I will take a look at where I can help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants