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

Add warning when postinstall fails (NPMv2 issue) #98

Closed
coopy opened this issue Dec 9, 2015 · 8 comments
Closed

Add warning when postinstall fails (NPMv2 issue) #98

coopy opened this issue Dec 9, 2015 · 8 comments
Assignees

Comments

@coopy
Copy link
Contributor

coopy commented Dec 9, 2015

Consider removing postinstall step in order to open up npm 2 compatibility again.

#95 removed Node versions other than 5 from the Travis run, in order to ensure npm v3 is used in the install step. This is because of FormidableLabs/builder#35.

This has consequences – ​_not_​ doing postinstall means you must:

  1. give up git dependencies / installations; OR
  2. have to commit lib to source

There is a lot more detail in FormidableLabs/formidable-react-component-boilerplate#56.

@coopy coopy added this to the Beta milestone Dec 9, 2015
@coopy
Copy link
Contributor Author

coopy commented Dec 10, 2015

@ryan-roemer has a strawman solution for this:

  1. Underlying archetype (builder-victory-component) swaps out the existing npm:postinstall shell invocation with cd lib || builder run build-lib (to simplify)
  2. Each implementing ROOT project (and the boilerplate) updates the postinstall script in package.json to say cd lib || builder run npm:postinstall.

This will take care of the problem with builder not being available for any project that's being npm installed using a bad npmv2. This will not take care of the problem when git installing or git cloneing, since lib/ isn't part of the repo.

@coopy
Copy link
Contributor Author

coopy commented Dec 17, 2015

Potential compromise: Can we prompt user to upgrade to npmv3 if the postinstall fails in this way?

@ryan-roemer
Copy link
Member

Potential compromise: Can we prompt user to upgrade to npmv3 if the postinstall fails in this way?

That's potentially a lot of tooling/work that needs to (1) be Windows / Mac compatible and (2) can't rely on any npm installs to have succeeded because of the very nature of the problem.

I think with the cd lib || builder run npm:postinstall protection solving all of "from npm" installs, the folks who are doing a git install can probably do just fine with a README.md warning or such (?)

@ryan-roemer
Copy link
Member

@coopy -- I take that back. Amazingly this works on Mac and Windows (tried XP):

cd lib || builder run npm:postinstall || echo 'POSTINSTALL FAILED: If using npm v2, please upgrade to npm v3. See bug https://github.com/FormidableLabs/builder/issues/35' && exit 1

But, you'll need to update all the Victory projects to have this exact string (it can't be abstracted to builder, because in our failure scenario builder / archetype didn't install 😉 )

@coopy
Copy link
Contributor Author

coopy commented Dec 17, 2015

Awesome, thanks for testing that @ryan-roemer!

@coopy coopy changed the title Review NPM v2 incompatibility situation (removing postinstall?) Add warning when postinstall fails (NPMv2 issue) Dec 17, 2015
@knowbody
Copy link
Contributor

knowbody commented Jan 4, 2016

can we go forward and add this warning to all the victory components?

@coopy
Copy link
Contributor Author

coopy commented Jan 4, 2016

Yes.
mån 4 jan. 2016 kl. 07:24 skrev Mateusz Zatorski [email protected]:

can we go forward and add this warning to all the victory components?


Reply to this email directly or view it on GitHub
#98 (comment)
.

knowbody added a commit to FormidableLabs/victory-scatter that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-bar that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-chart that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-line that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-axis that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-label that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-pie that referenced this issue Jan 4, 2016
knowbody added a commit to FormidableLabs/victory-animation that referenced this issue Jan 4, 2016
@knowbody
Copy link
Contributor

warnings are added now, so closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants