ds300 / patch-package

Fix broken node modules instantly 🏃🏽‍♀️💨
MIT License
10.36k stars 290 forks source link

Support for GitHub repositories without a package.json #163

Open DimitarNestorov opened 5 years ago

DimitarNestorov commented 5 years ago

I'm using yarn to manage some native iOS dependencies by simply adding them:

yarn add https://github.com/SnapKit/SnapKit#4.2.0

But when I try to patch the package: image

It would be awesome if the tool could check if a package was downloaded from GitHub. Here's how it looks in package.json: image

Note: A tag/commit/branch is not required to be specified inside the package.json in which case patch-package wouldn't be able to show the patch warning when packages get updated without tapping into the lockfile.

ds300 commented 5 years ago

haha interesting. I had no idea yarn could install any old git repo like that.

To be honest I'm probably not going to support this unless it becomes a common usage pattern.

DimitarNestorov commented 5 years ago

npm can do that too

benallfree commented 4 years ago

Came here for this too. Not all packages are published on npm... Example: https://github.com/tightenco/ziggy

It even has a package name that conflicts with an existing npm package.

$ patch-package https://github.com/tightenco/ziggy\#master

...would be the way to get it unless patch-package ziggy was smart enough to find ./node_modules/ziggy and resolve it against ./package.json's ziggy entry.

Probably a fringe case, but still :)

ds300 commented 3 years ago

I'm gonna leave this open as it's a valid feature that I probably won't get around to. If any contributors come up with a way to do this with good UX then I'm happy to accept pull requests.

For me the trickiest problem is: how do we get the version of the package? Is there always a git ref in the url? should we use the commit sha as a version string to be safe?

peey commented 3 years ago

The issue is fixed for me (I use npm + package-lock) through this patch

diff --git a/node_modules/patch-package/dist/getPackageResolution.js b/node_modules/patch-package/dist/getPackageResolution.js
index bc7ffaa..287a2f1 100644
--- a/node_modules/patch-package/dist/getPackageResolution.js
+++ b/node_modules/patch-package/dist/getPackageResolution.js
@@ -65,7 +65,7 @@ function getPackageResolution({ packageDetails, packageManager, appPath, }) {
         lockFileStack.reverse();
         const relevantStackEntry = lockFileStack.find((entry) => entry.dependencies && packageDetails.name in entry.dependencies);
         const pkg = relevantStackEntry.dependencies[packageDetails.name];
-        return pkg.resolved || pkg.from || pkg.version;
+        return pkg.resolved || pkg.version || pkg.from;
     }
 }
 exports.getPackageResolution = getPackageResolution;

Which corresponds to here in the source: https://github.com/ds300/patch-package/blame/5c2c92bf504885fba4840870a23fc8999c00e572/src/getPackageResolution.ts#L94

Works for me, but I'm not sure what else this breaks though. Perhaps pkg.from should be completely removed because I didn't see an example of when that field had a useful value in my package-lock.json. That was also the issue: it contains a invalid value that npm can't install.

I'm now also left wondering if patch-package is smart enough to patch itself before attempting to patch other things till this or some other fix makes it to a released version :P