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.

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

Install scripts not triggered by 'yarn add' #1238

Closed
thebuilder opened this issue Oct 19, 2016 · 7 comments
Closed

Install scripts not triggered by 'yarn add' #1238

thebuilder opened this issue Oct 19, 2016 · 7 comments

Comments

@thebuilder
Copy link

Do you want to request a feature or report a bug?
feature

What is the current behavior?
preinstall and postinstall scripts are not run after using yarn add

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior?
When using npm the scripts are triggered when you add a new package with npm install.
Yarn triggers it when doing yarn or yarn install, but not when you add new package.
This would also be an issue if you are using other commands like yarn upgrade, that can install new versions.

I think it should follow the same logic - I'm using a postinstall script in a simple project, to build a Webpack DLL file, which should be triggered whenever packages change.

Please mention your node.js, yarn and operating system version.
Yarn: 16.0.0
Node: 6.8.0
Mac Sierra

@darahak
Copy link

darahak commented Oct 19, 2016

Looks like it will be solved with this change: #800

@thebuilder
Copy link
Author

#800 correctly triggers the install script when using yarn - This is to also enable it for yarn add

@darahak
Copy link

darahak commented Oct 19, 2016

Oh you're right, I missed that detail.
I guess add.js needs to run executeLifecycleScript() like in install.js.

Not sure if it's enough to make it work though. I'm discovering the project.

@samccone
Copy link
Member

Fixed in #800 please reopen if not.

Thanks for the bug!

@thebuilder
Copy link
Author

#800 is only a partial fix. As @darahak suggests, the executeLifeCycleScript() in add.js should match install.js.

@samccone
Copy link
Member

ah yes you are right, sorry was too eager in closing.

Any interest in opening a PR to resolve this?

@darahak
Copy link

darahak commented Oct 29, 2016

Resolved with #1422 👍

@wyze wyze closed this as completed Oct 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants