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.

Sign up for GitHub

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

Jump to bottom

feat: add preliminary support for Bun #490

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

feat: add preliminary support for Bun #490

wants to merge 8 commits into from

Conversation

Strengthless
Copy link

@Strengthless Strengthless commented Sep 10, 2023 •
edited

Closes #489.

Changelogs:

  • Added --use-bun flag, similar to the current --use-yarn flag.
  • Added parseBunLockfile.ts to convert bun.lockb into yarn v1 lockfiles.
  • Updated getPackageResolution.ts to handle bun conversions before parsing.
  • Updated detectPackageManager.ts to detect bun, bun workspaces and multiple lockfiles.
  • Updated README with setup instructions for Bun.

Caveats:

  • If user has multiple lockfiles, patch-package uses npm by default. However, in the rare case where only bun.lockb and yarn.lock files exist, we have to resort to using yarn. This behaviour has been documented in the CLI and README (see 0491b91).
  • Currently awaiting confirmation on how Bun handles postinstalls, for writing up the README about bun setup. (Will bun remove execute postinstall scripts? If not, bun users should also install postinstall-postinstall just like yarn v1 users.)
  • Currently awaiting confirmation on how Bun workspaces work, for writing up the README about bun workspaces setup (Is it similar to yarn workspaces, where users need to "repeat the setup process for the child package" if they want to patch un-hoisted packages?)

Temporary workaround (while this PR is unmerged):

  • Citing support patching dependencies, similar to pnpm patch/patch-package oven-sh/bun#2336 (comment),

    Attempting to create a patch using bunx patch-package <package-name> fails.

    However, if you have pre-existing patches in the ./patches directory, you can successfully apply them using the bunx patch-package command (or via package.json -> scripts -> "postinstall": "patch-package")

    As a temporary workaround to generate a patches/package+name+version.patch file

    • run bun install --yarn it just converts bun.lockb into a yarn.lock. (or bun ./bun.lockb > ./yarn.lock)
    • make the necessary modifications inside node-modules/<package-name>
    • bunx patch-package <package-name>
    • rm yarn.lock
    • package.json -> scripts -> "postinstall": "bunx patch-package"
      https://github.com/ds300/patch-package

@OnurGvnc
Copy link

I tried bun remove <package-name> and saw that it executes the postinstall script.

❯ bun remove zod
bun remove v1.0.0 (822a00c4)
patch-package 8.0.0
Applying patches...
@remix-run/dev@1.19.3 ✔
@remix-run/react@1.19.3 ✔
 - zod

 1 packages removed [38.00ms]

@Strengthless
Copy link
Author

Strengthless commented Sep 10, 2023 •
edited

This PR is now ready to merge, with only two caveats as mentioned in #490 (comment).

@ds300 Can you please review this PR? :)

@talhabalaj talhabalaj mentioned this pull request Sep 14, 2023
Closed
@robertherber
Copy link

Looking forward to this :) Been using pnpm for a while and been missing patch-package - so having it work with bun will be yummy!

Hoping this will work in monorepos as well!

@lovlyx
Copy link

lovlyx commented Jan 6, 2024

any news on this?

@rikur
Copy link

rikur commented Jan 12, 2024

Wish I could use patch-package to patch patch-package with this patch 😂 I guess I just need to fork the repo.

@robertherber
Copy link

Until this is merged, would you consider publishing this fork on npm @Strengthless?

@erickreutz
Copy link

LGTM!

@medv
Copy link

medv commented Feb 9, 2024 •
edited

It's happening!

Using this method for now oven-sh/bun#2336 (comment)

@robertherber
Copy link

@ds300 I understand you probably have lots going on - but having this merged would be a key enabler! 🙏🙂

@miblanchard
Copy link

Until this is merged, would you consider publishing this fork on npm @Strengthless?

@robertherber I am trying to use this method right now and published it under @miblanchard/patch-package@8.0.12 until this is merged and published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for Bun
8 participants
@Strengthless @OnurGvnc @robertherber @lovlyx @rikur @erickreutz @medv @miblanchard