Add yarn berry compatibility

[maman]

Summary

Yarn "berry" introduced a new lockfile format that now adheres to YAML.

The current patch-package's yarn lockfile parser uses @yarn/lockfile - which will throw errors since it's only valid for yarn v1 (or "classic").

This PR adds support to parse the new, yaml-based lockfile. The feature detection is performed by checking the existence of the string # yarn lockfile v1 on the lockfile. If the string is found, then we'll parse it using @yarn/lockfile. if it isn't, then we'll use the yaml package to parse it.

Another thing that we need to adjust is, on the yarn berry lockfile packages from npm are prefixed with "npm:". This PR also adjusts it to remove the prefix, leaving the version number only as expected.

[d0whc3r]

What about this?

[d0whc3r]

btw, --create-issue isn't working with these changes and yarn 3

[d0whc3r]

It worked for me if I add an extra change, in makePatch.ts, before running the install command, https://github.com/ds300/patch-package/blob/master/src/makePatch.ts#L130, I added the "yarn set version classic" command like:

        spawnSafeSync(`yarn`, ["set", "version", "classic"], {
          cwd: tmpRepoNpmRoot,
          logStdErrOnError: false,
        })

With this change, it works

[d0whc3r]

These are the changes (in build files):

diff --git a/node_modules/patch-package/dist/getPackageResolution.js b/node_modules/patch-package/dist/getPackageResolution.js
index bc7ffaa..969414a 100644
--- a/node_modules/patch-package/dist/getPackageResolution.js
+++ b/node_modules/patch-package/dist/getPackageResolution.js
@@ -9,6 +9,7 @@ const PackageDetails_1 = require("./PackageDetails");
 const detectPackageManager_1 = require("./detectPackageManager");
 const fs_extra_1 = require("fs-extra");
 const lockfile_1 = require("@yarnpkg/lockfile");
+const yaml_1 = require("yaml");
 const find_yarn_workspace_root_1 = __importDefault(require("find-yarn-workspace-root"));
 const getPackageVersion_1 = require("./getPackageVersion");
 function getPackageResolution({ packageDetails, packageManager, appPath, }) {
@@ -24,12 +25,25 @@ function getPackageResolution({ packageDetails, packageManager, appPath, }) {
         if (!fs_extra_1.existsSync(lockFilePath)) {
             throw new Error("Can't find yarn.lock file");
         }
-        const appLockFile = lockfile_1.parse(fs_extra_1.readFileSync(lockFilePath).toString());
-        if (appLockFile.type !== "success") {
-            throw new Error("Can't parse lock file");
+        const lockFileString = fs_extra_1.readFileSync(lockFilePath).toString();
+        let appLockFile;
+        if (lockFileString.includes("yarn lockfile v1")) {
+          const parsedYarnLockFile = lockfile_1.parse(lockFileString)
+          if (parsedYarnLockFile.type !== "success") {
+            throw new Error("Can't parse lock file")
+          } else {
+            appLockFile = parsedYarnLockFile.object
+          }
+        } else {
+          try {
+            appLockFile = yaml_1.parse(lockFileString)
+          } catch (e) {
+            console.error(e)
+            throw new Error("Can't parse lock file")
+          }
         }
         const installedVersion = getPackageVersion_1.getPackageVersion(path_1.join(path_1.resolve(appPath, packageDetails.path), "package.json"));
-        const entries = Object.entries(appLockFile.object).filter(([k, v]) => k.startsWith(packageDetails.name + "@") &&
+        const entries = Object.entries(appLockFile).filter(([k, v]) => k.startsWith(packageDetails.name + "@") &&
             v.version === installedVersion);
         const resolutions = entries.map(([_, v]) => {
             return v.resolved;
@@ -49,6 +63,9 @@ function getPackageResolution({ packageDetails, packageManager, appPath, }) {
         if (resolution.startsWith("file:.")) {
             return `file:${path_1.resolve(appPath, resolution.slice("file:".length))}`;
         }
+        if (resolution.startsWith("npm:")) {
+          return resolution.replace("npm:", "")
+        }
         return resolution;
     }
     else {
diff --git a/node_modules/patch-package/dist/makePatch.js b/node_modules/patch-package/dist/makePatch.js
index 985589e..e8704dd 100644
--- a/node_modules/patch-package/dist/makePatch.js
+++ b/node_modules/patch-package/dist/makePatch.js
@@ -67,6 +67,10 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
         if (packageManager === "yarn") {
             console.info(chalk_1.default.grey("•"), `Installing ${packageDetails.name}@${packageVersion} with yarn`);
             try {
+              spawnSafe_1.spawnSafeSync(`yarn`, ["set", "version", "classic"], {
+                  cwd: tmpRepoNpmRoot,
+                  logStdErrOnError: false,
+              });
                 // try first without ignoring scripts in case they are required
                 // this works in 99.99% of cases
                 spawnSafe_1.spawnSafeSync(`yarn`, ["install", "--ignore-engines"], {

Also I added yaml as a dependency of my own project, just to have it in node_modules

[milesj]

Any movement on this?

[FilDevTronic]

@ds300 are you still maintaining this project?

[ShaneZhengNZ]

@ds300 are you still maintaining this project?

[milesj]

Yarn 2/3 has built-in patching, you don't need this package: https://yarnpkg.com/cli/patch

[FilDevTronic]

Yarn 2/3 has built-in patching, you don't need this package: https://yarnpkg.com/cli/patch

Sure - if you don't want the features of patch-package then go use the in-built yarn patch. This isn't news.

[milesj]

@FilipMaslovaric It may be news to people who are unaware that Yarn supports this natively now.

Furthermore, yarn patches and the patches produced by patch-package are not the same, so you run the risk of mutating yarn's node modules in an incorrect way. Just letting people know, no need to be snarky.

[FilDevTronic]

@FilipMaslovaric It may be news to people who are unaware that Yarn supports this natively now.

Furthermore, yarn patches and the patches produced by patch-package are not the same, so you run the risk of mutating yarn's node modules in an incorrect way. Just letting people know, no need to be snarky.

The issues of the two not producing compatible changes is a non sequitur to what I said.

Obviously it's news if you didn't know about it, but in and of itself, isn't news. Sorry you just found out.

It's misleading to say that you "don't need" patch-package simply because Yarn has its own yarn patch, because you're assuming that everyone is happy to forego the features that patch-package has, that yarn patch does not.

You're may mislead others with what you're "just letting [them] know".

Further clarification (29/09/2022):

The reason why you absolutely should be using patch-package over yarn patch if you can, is that yarn patch doesn't concern itself with telling you if a change in the package being patched may have invalidated the patch itself, or inversely if your patch is now going to break things. patch-package in @ds300 wisdom, does this very useful thing, and will scream at you in your console if the patch is now incompatible since the package was updated, or even if there was just an update, but the patch doesn't seem to be incompatible (nudging you to double-check, and just update the version of the package that the patch file is pointing to).

If you happen to be in a situation where your project is moving faster than the community can patch or properly fix open source libraries, or even if some other internal dependent project isn't quite ready to publish changes, and you are patching things relatively frequently as an interim-solution, then this package is an absolute lifesaver as it can be easier to forget that something was indeed patched when the evidence of said patch is buried in a somewhat large package.json file whose section you may not visit very often in your daily work.

Would love for yarn patch to adopt some of these features, as going with first-party solutions that are more integrated generally "should" be a better choice, but in this instance, this little gem is far superior.

[kylerjensen]

@ds300 could you merge this PR please?

[orta]

Cool, thanks 👍🏻

[FilDevTronic]

@orta you are a life saver. Thank you. 🙇 @maman thanks for getting us this fix 🙏

[kylerjensen]

@orta Thanks for merging this PR! Do you know when we can expect a new version to be published to NPM?

[ds300]

This was just released in v6.5.0! Thanks for your contribution 🙏🏼 🎉