Skip to content

Changing an existing patch is problematic in some situations #557

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

Open
JonMarbach opened this issue Feb 20, 2025 · 3 comments
Open

Changing an existing patch is problematic in some situations #557

JonMarbach opened this issue Feb 20, 2025 · 3 comments

Comments

@JonMarbach
Copy link

Hey there! I'm so glad patch-package exists - it is really helping me with a problematic package that I needed to fix urgently. But I ran into a problem... I created a patch, it got picked up by my team and our build servers, and then after a few weeks I realized I needed to refine the patch. I had seen this in the readme:

"Updating patches
Use exactly the same process as for making patches in the first place, i.e. make more changes, run patch-package, commit the changes to the patch file."

But what I experienced is not so carefree as this makes it sound. When I published the new version of the patch, all of a sudden other machines were getting this error:

ERROR Failed to apply patch for package

And there wasn't much to go on as far as what the problem was (I see some todos in the code about providing more information about why something failed - that would be good to do.)

Fast-forwarding, I finally realized that because the other developers/servers had applied that first patch, their copies of node_modules were modified and therefore, the patch integrity check fails. This is also a problem for some build servers which cached copies of node_modules instead of running "fully clean" builds. So, the only recourse has been to notify our whole development team to remove their node_modules folder.

I think it would at least be good to mention this in your readme RE Updating patches to offer more cautious advice. And of course, it would be really awesome to have something in the implementation that allows for at least a workaround if not something official, like a pre-patch-process which reverts the old patch before applying the new one. And of course, it would be great to get more info about a failure when it happens.

Thanks very much!

@mlazari
Copy link

mlazari commented Mar 16, 2025

I have a similar issue trying to patch a package (although I never encountered it for other packages I patched in the past). In my case if I remove node_modules then run npm install then it works without error (but doesn't apply the patch completely). However if I run npm install one more time without removing node_modules it fails for that package (I suspect it's related to the patch not being applied completely first time).
Minimal repro: https://github.com/mlazari/patch-package-repro

L.E. I think it's related to the patch adding lines to the end of a file. It looks like there is some bug in that case, each time you re-run npm install it tries to add something to the end of the existing patched file (see https://drive.google.com/file/d/1d1eHyzvQhYGTX-XOCt0RnOS8AhjcwXR1/view).

@lppedd
Copy link

lppedd commented Mar 18, 2025

Folks, what about checking if the latest change set - matched against a stored hash - contains changes to patches?
This could be done via preinstall, and if there are changes to patches we first delete the offending packages under node_modules.

@mlazari
Copy link

mlazari commented Mar 18, 2025

@lppedd For some patches (like adding a few lines at the end of a file) it doesn't patch correctly even when installing for the first time after deleting node_modules (see the video in my previous comment). I suspect it might be related to what @JonMarbach experiences, although I am not sure. Usually running npm i multiple times and thus applying the patch multiple times is idempotent / doesn't change something in the file every time, but in this case as you see it in the video it is not working properly.

My issue might be related to this: #430 (I didn't try adding lines to the beginning of the file though, and I am using npm, not yarn)

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

3 participants