Skip to content

Commit

Permalink
fix(ci): move patch-package to prebump
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sec-ant
Sec-ant committed Jan 22, 2024
1 parent 6817e13 commit dad351c
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 2 deletions.
1 change: 0 additions & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
"access": "public"
},
"scripts": {
"postinstall": "patch-package",

This comment has been minimized.

Sign in to view

Copy link
@JounQin

JounQin Jan 23, 2024

I think it should be in prepare lifecycle.

This comment has been minimized.

Sign in to view

Copy link
@Sec-ant

Sec-ant Jan 23, 2024 •
edited

Author Owner

The README of patch-patch package suggests postinstall so that was my first attempt. It works good locally, but the codesandbox-ci has some writing permission issues, so I moved it to prebump.

prepare will also be triggered after install, so I assume it won't help solve the codesandbox-ci issue?

This comment has been minimized.

Sign in to view

Copy link
@JounQin

JounQin Jan 23, 2024 •
edited

postinstall is actually wrong because it will also be executed on users installation of this package. prepare is the perfect replacement. For yarn berry, you can even use https://github.com/un-es/yarn-plugin-prepare-lifecycle

prepare will also be triggered after install, so I assume it won't help solve the codesandbox-ci issue?

I have never that issue for all my projects using Codesandbox CI. And install with prepare with patch-package installed is much expected? Otherwise if you have other patches instead of bumpp, how could it be handled?

image

Secondly, I don't understand why there is a rename action. (Maybe that's why Codesandbox CI breaks)

Last but not least, you should migrate to other tools instead of hacking: #55

This comment has been minimized.

Sign in to view

Copy link
@Sec-ant

Sec-ant Jan 23, 2024

Author Owner

postinstall is actually wrong because it will also be executed on users installation of this package. prepare is the perfect replacement.

Good to know that, thanks!

Secondly, I don't understand why there is a rename action. (Maybe that's why Codesandbox CI breaks)

That's because I changed the source code and built it locally and replaced the dist folder. Patching the dist directly is painful.

Last but not least, you should migrate to other tools instead of hacking

Yeah this fix was merged but has not been released yet, so I patched it locally. I've tried changeset but the workflow doesn't fit me well. I'm still trying multiple tools to find the one I like~

Anyway, prebump should be ok?

This comment has been minimized.

Sign in to view

Copy link
@JounQin

JounQin Jan 23, 2024

Anyway, prebump should be ok?

Only for this case for bumpp, it's not a general solution.

This comment has been minimized.

Sign in to view

Copy link
@Sec-ant

Sec-ant Jan 23, 2024 •
edited

Author Owner

fixed in #70
Seems a rename issue 👍
"prepare": "husky install",

This comment has been minimized.

Sign in to view

Copy link
@JounQin

JounQin Jan 23, 2024

https://github.com/toplenboren/simple-git-hooks

This tool is much easier to be used as husky@v4.

This comment has been minimized.

Sign in to view

Copy link
@Sec-ant

Sec-ant Jan 23, 2024

Author Owner

Will try that, thanks!
"prebump": "patch-package",
"bump": "bumpp",
"dev": "vite",
"lint": "biome lint .",
Expand Down

0 comments on commit dad351c

Please sign in to comment.