Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Please remove postinstall #36

Closed
Dignifiedquire opened this Issue · 13 comments

3 participants

@Dignifiedquire

I've got error reports coming in from all over, that postinstall isn't working (e.g. vojtajina/testacular#46). I know it should and all and I'll look into filling an issue with npm but as per the offical npm documentation here:

NOTE: INSTALL SCRIPTS ARE AN ANTIPATTERN
tl;dr Don't use install. Use a .gyp file for compilation, and prepublish for anything else.

it shouldn't be used for this anyway.

Please look into this. I'm happy to help out if neeed.

@paulmillr
Owner
  1. I don’t want to store JS output in repo. I / pull requesters forgot to compile js every time etc.
  2. Prepublish is already done. Chokidar is compiled before publishing to npm.
  3. Postinstall checks if there is lib dir and drops execution if yes. This makes chokidar not dependent on coffeescript. It worked for like one year for me. So it’s definitely npm issue.

Any thoughts on how can we not store compiled js in repo w/o postinstall?

@paulmillr
Owner

It’s pretty easy to node maintainers to said install scripts are antipattern and shit, when no one uses coffeescript or any compiled-to-js language. :-1:

@vojtajina

Agree, that you don't wanna have js code in the repo and you don't wanna force anyone to compile it.

The compilation should happen during "publish to npm" task. The published npm module should only contain the js files (no coffee, no tests).

And AFAIK, the coffee->js compilation already happens during publish (you might add test/* and src/* to .npmignore), so I believe the whole postinstall script is not required at all.

@vojtajina

To make simpler, you can have some grunt task for release, that's what we do with Testacular, so whole release is just a single command: https://github.com/vojtajina/testacular/blob/master/tasks/npm.js

@paulmillr
Owner

@vojtajina I have git release for this. nice, efficient, simple.

You’re right about compilation, it already happens. postinstall task is only for folks who use git release that is not on npm.

We can remove postinstall step before publishing to npm but i’m not sure how can this be automated.

@Dignifiedquire

@paulmillr What about you add something like this

var fs = require('fs');
var pkg = JSON.parse(fs.readFileSync('package.json'));

delete pkg.scripts.postinstall;
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2));

to your release process?

@vojtajina

Well, I would do:

  • lib/* in .gitignore
  • src and test in .npmignore

Add prepublish (see https://npmjs.org/doc/scripts.html) into package.json, that will do the compilation.

@paulmillr paulmillr closed this in 75ce3ba
@paulmillr
Owner

Okay, .postinstall is now automatically removed on publish and brought back after publish. 0.5.2 without .postinstall is on npm.

@Dignifiedquire

Thanks a lot!

@paulmillr
Owner

FUCK. postpublish is executed but on npm cached copy. npm/npm#2315 npm/npm#2550

@paulmillr
Owner

please vote for my shit npm/npm#3053

@iki iki referenced this issue in brunch/brunch
Merged

Fix installing from a git repository #546

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.