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

makePatch.js does not actually npm install the version that it says it is #320

Open
jeffory-orrok opened this issue May 22, 2021 · 1 comment

Comments

@jeffory-orrok
Copy link

jeffory-orrok commented May 22, 2021

Show of hands -- how many people have run patch-package on itself???

cat >patches/patch-package+6.4.7.patch
diff --git a/node_modules/patch-package/dist/makePatch.js b/node_modules/patch-package/dist/makePatch.js
index 985589e..326fae2 100644
--- a/node_modules/patch-package/dist/makePatch.js
+++ b/node_modules/patch-package/dist/makePatch.js
@@ -87,7 +87,7 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
             try {
                 // try first without ignoring scripts in case they are required
                 // this works in 99.99% of cases
-                spawnSafe_1.spawnSafeSync(`npm`, ["i", "--force"], {
+                spawnSafe_1.spawnSafeSync(`npm`, ["i", "--force", `${packageDetails.name}@${packageVersion}`], {
                     cwd: tmpRepoNpmRoot,
                     logStdErrOnError: false,
                     stdio: "ignore",
@@ -96,7 +96,7 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
             catch (e) {
                 // try again while ignoring scripts in case the script depends on
                 // an implicit context which we havn't reproduced
-                spawnSafe_1.spawnSafeSync(`npm`, ["i", "--ignore-scripts", "--force"], {
+                spawnSafe_1.spawnSafeSync(`npm`, ["i", "--ignore-scripts", "--force", `${packageDetails.name}@${packageVersion}`], {
                     cwd: tmpRepoNpmRoot,
                     stdio: "ignore",
                 });
@FibreFoX
Copy link

Not sure I can follow this patch ... the dependency does not get installed via a CLI parameter, but from a temporary created package.json which contains the version of that package.
See

[packageDetails.name]: getPackageResolution({

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

No branches or pull requests

2 participants