-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add pre and post install script calls #800
Add pre and post install script calls #800
Conversation
Also #721 which might be a dupe. |
Note that npm means to deprecate how prepublish currently works in npm 4.0, to be released this month: npm/npm#10074 |
@Kovensky Nice catch! After reviewing conversation there, it looks like it's still pretty up in the air what npm plans to do exactly to change the behavior. I'd be up for porting whatever they implement if that's desired, but these changes at least get us closer to npm compatibility (and resolves issues for a few people migrating). That said, I'm happy to remove |
@FLGMwt awesome work on this! IMHO, not having these hooks is a show stopper to a lot of projects using yarn. I would say feature parity with npm is what you should be shooting for, and having prepublish work the same way is probably best. |
Unless every single lifecycle script functions exactly the same as it does on npm, then yarn is not a drop-in replacement for npm - which I understand to be the goal. That includes prepublish's awful behavior. |
@kittens Curious on when this fix will get pushed to a release. This fix is going to fix a lot of issues with modules that depends on these hooks |
Just a note: This patch runs I noticed it because I (ab)used this behavior to prevent accidental |
@ljharb A beer says that prepublish isn't that big a problem, and this is a good way to find out. |
@dominictarr if you're right, I will buy you a whole case. But if there are non-zero packages that depends on it, that would be enough for it to be a huge problem. |
in the browser world, not everything is completely compatible, and, well, it's annoying some times, but we survive. I wasn't around at the time when javascript was brand new... but I'm gonna hazard to suggest that "mutating" a little bit, could actually make something better. And in any case, there isn't a spec for npm just an implementation, however if there are more than one implementation, maybe it's time to start thinking about a spec. (maybe wait another 15 minutes though) |
@dominictarr on point.. since there are no specs let's rethink some of the procedures and not blindly copy stuff from npm.. just because. |
All the things browsers agree upon are the spec - that's true in the JS standard as well as in browser standards. Since |
Any idea when we can expect the next release so we can rely on this functionality? |
Any update on the status of a release including this? |
I am wondering if this also works for the 'yarn-offline-mirror' feature? Some clarification would be very helpful to me! |
Yarn doesn't install selenium webdriver when installing node module |
Hello, I am testing the I have forked a package and am trying to install it from GitHub:
This
This is not being run during the installation. I would expect this to be run, since GitHub is only for source code, and people don't usually check in the build artifacts. I just read through this issue on npm, and it looks like npm also doesn't run the Or should I just add this step as a EDIT: Ah, sorry, I just realized that this is for the |
@ndbroadbent: is |
@jamesjnadeau I tried adding a However, So I guess the only way is to push a new branch with the build artifacts checked in? |
Summary
Adds calls to the following named scripts:
preinstall
,install
,postinstall
and wrapsprepublish
in anif !prod
, as it is innpm install
Should resolve #783 Happy to discuss.Test plan
Given the following package.json:
npm install
outputs the following:yarn
, however only executesprepublish
(before changes):After changes:
and with
--prod
flag:I'm also working through how to add unit tests for this and can hold merge for that is desired.