-
Notifications
You must be signed in to change notification settings - Fork 257
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
chore(scripts): add prepare script #1812
Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@@ -76,6 +76,7 @@ | |||
"test:build": "vitest --mode test-build", | |||
"tsd": "tsc -p test-dts/tsconfig.tsd.json", | |||
"build": "rollup -c rollup.config.js", | |||
"prepare": "rollup -c rollup.config.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're duplicating build
script instead of pnpm run build
- we do not want to rely on pnpm
, and having build
invoked as pnpm run prepare
feels very wrong to me
I don't have a strong opinion about this one, I don't think I ever used @lmiller1990 @freakzlike WDYT? |
The great thing is it used automatically and our users will be able to use main branch if needed |
I think this is good. Not sure if that will affect the release process |
@freakzlike not at all. We have extra build triggered inside pipeline (you can see even in this PR that |
Let's go ahead and try it then |
This is extremely simple, yet useful (in my opinion) change.
Since npm v5 npm executes
prepare
script when dependency is installed from git. By includingprepare
script it will allow our users use@vue/test-utils
frommain
(or any other branch) even if release was not published to npm yetHowever, this has a downside - after doing install locally (without include
@vue/test-utils
as dependency) it will be also executed. However, I feel here benefits outweight the downsideWDYT?