Skip to content

feat: patch package#4885

Merged
zkochan merged 4 commits into
mainfrom
patch
Jun 19, 2022
Merged

feat: patch package#4885
zkochan merged 4 commits into
mainfrom
patch

Conversation

@zkochan

@zkochan zkochan commented Jun 13, 2022

Copy link
Copy Markdown
Member

TODO:

  • when a package is patched, the side-effects cache key of that package should contain the patch file hash.
  • patching should not affect files in the CAFS (so maybe it should not work when hard links are used?)

For creating the patch, we may probably use this library of Yarn: https://github.com/yarnpkg/berry/blob/82376c1b7b96ea894ca92a9c8b777bbc17535ea3/packages/plugin-patch/sources/patchUtils.ts#L279

@Jack-Works

Copy link
Copy Markdown
Member

Love to see this work 👀

Does it contain workspace support?

@zkochan

zkochan commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

Yes

@zkochan

zkochan commented Jun 17, 2022

Copy link
Copy Markdown
Member Author

I will probably be able to implement it only with some restrictions. I don't think we can allow publishing packages with patched dependencies. pnpm cannot have two copies of the same package (with the same version) inside one node_modules (unless that package has peer dependencies).

If we allow packages to be published with patched dependencies, then pnpm will have to be able to write different variations of the same package, as it may be patched in one of the dependencies and not patched in some other dependencies.

I can implement something like the following, that you would put to the package.json of your project:

{
  "patchedDependencies": {
    "express@3": "patches/express.patch"
  }
}

and it would patch all express v3 packages in the node_modules.

@Jack-Works

Copy link
Copy Markdown
Member

and it would patch all express v3 packages in the node_modules.

patch-package requires the version is pinned. Allowing this board version might fail to patch the package at some time. (conflict changes)

@zkochan

zkochan commented Jun 17, 2022

Copy link
Copy Markdown
Member Author

OK, the version can be pinned. But that version of the dependency will have to be patched in subdependencies as well. Not only when it is a direct dependency.

@zkochan zkochan force-pushed the patch branch 3 times, most recently from 9c08baa to 0c216bb Compare June 18, 2022 00:35
@zkochan zkochan marked this pull request as ready for review June 18, 2022 00:48
@zkochan zkochan force-pushed the patch branch 3 times, most recently from ae2d212 to e05debe Compare June 18, 2022 20:00
@zkochan zkochan added this to the v7.4 milestone Jun 18, 2022
@zkochan

zkochan commented Jun 18, 2022

Copy link
Copy Markdown
Member Author

I'll add the patch and patch-commit commands in a separate PR.

@zkochan zkochan requested a review from a team June 18, 2022 22:49

@Jack-Works Jack-Works left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In patch-package there is a really useful feature, you can modify packages in node_modules directly, and then run npx patch-package PACKAGE_NAME, it will compare the modified package with the original one and generate a patch file.
I'm wondering if pnpm is going to have this too. 👀

@zkochan

zkochan commented Jun 19, 2022

Copy link
Copy Markdown
Member Author

No, it will work as in Yarn with the patch and patch-commit commands.

Editing files inside node_modules is problematic because you would change the file in the store as well, when the file is a hard link.

@zkochan zkochan merged commit 2a34b21 into main Jun 19, 2022
@zkochan zkochan deleted the patch branch June 19, 2022 08:44
@Jack-Works

Copy link
Copy Markdown
Member

No, it will work as in Yarn with the patch and patch-commit commands.

Just read the document of yarn, I found it is better!

@zkochan

zkochan commented Jun 19, 2022

Copy link
Copy Markdown
Member Author

Next PR in progress: #4900

@renovate renovate Bot mentioned this pull request Jun 28, 2022
1 task
@renovate renovate Bot mentioned this pull request Aug 1, 2022
1 task
@theprojectsomething

Copy link
Copy Markdown

Super useful feature, thanks! Have recently been trialing this in a monorepo workflow, couple of issues:

  • am finding that the temporary package created for patching contains the compiled dist only. Is this down to per-package configs or how pnpm works?
  • Attempting to patch an org-owned package at an application-level, prior to upstreaming (i.e. that should remain un-patched in other monorepo applications). Assume this is a fairly common requirement. Open to aliases, hoisting overrides, postinstall solutions. Is this possible?

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

Successfully merging this pull request may close these issues.

3 participants