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

Should be using prepare rather than postinstall step for website dependency install #2165

Closed
benjamind opened this issue Oct 18, 2019 · 8 comments · Fixed by #2166
Closed
Labels

Comments

@benjamind
Copy link
Contributor

In the latest Mobx 5.14.1, Installing mobx as a dependency is resulting in the postinstall script executing which is trying to install the dependencies of the website folder which is not distributed in the package resulting in errors like this during dependency installation:

$ yarn --cwd website install
Killed
yarn install v1.16.0
error Directory "/home/circleci/project/node_modules/mobx/website" doesn't exist
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
error Command failed with exit code 1.

Reproduction:

  • install as a dependency of any project.

Easy fix is to change postinstall to prepare. prepare will execute only on local installation, and not during installation as a dependency.

@danielkcz
Copy link
Contributor

Thanks for the report, that's definitely bad and ugly bug. We will sort it out soon.

benjamind added a commit to benjamind/mobx that referenced this issue Oct 18, 2019
@danielkcz
Copy link
Contributor

Or you will, that's also an option :D

amcclain added a commit to xh/hoist-react that referenced this issue Oct 18, 2019
+ See mobxjs/mobx#2165 which is a blocker for allowing it to go to 5.14.1
@vnesek
Copy link

vnesek commented Oct 19, 2019

What about npm users, is using yarn requirement now?

@danielkcz
Copy link
Contributor

danielkcz commented Oct 19, 2019

No, these lifecycle scripts work the same across yarn & npm.

Point is that, postinstall is executed when installing mobx on your machine, but prepare is only for development of mobx.

@vnesek
Copy link

vnesek commented Oct 19, 2019

Running on a system without yarn installed:

user@local> npm i

> [email protected] postinstall /Users/vnesek/Workspace/ddex-front/node_modules/mobx
> yarn --cwd website install

sh: yarn: command not found

@benjamind benjamind mentioned this issue Oct 19, 2019
1 task
danielkcz pushed a commit that referenced this issue Oct 19, 2019
* Fixes #2165 - switches postinstall to prepare

* Bumps to v5.14.2 and updates changelog

* Removes version bump
@danielkcz
Copy link
Contributor

Published 5.14.2 ... note there is no 4.14.2 as MobX 4 wasn't affected.

@benjamind
Copy link
Contributor Author

@vnesek yarn has been required for mobx development for a long time now I think. It is not required to consume mobx as a dependency though.

@lock
Copy link

lock bot commented Dec 18, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants