This repository has been archived by the owner. It is now read-only.

run `prepublish` for git url packages #3055

Closed
isaacs opened this Issue Jan 14, 2013 · 81 comments

Comments

Projects
None yet
@isaacs
Member

isaacs commented Jan 14, 2013

After installing a git url package, perhaps the prepublish script should be run, since git repos will typically not keep around any built artifacts that the prepublish script generates.

However, prepublish scripts also typically require the use of devDependencies packages, so it might not be possible to do this in a good way. (In which case, it'll just be on the users of git url packages to run whatever scripts are necessary to make them ready to use.)

@dfellis

This comment has been minimized.

Show comment
Hide comment
@dfellis

dfellis Jan 15, 2013

I don't believe it's necessary. If the user has forked a project and is installing via a raw git repo, they could add a commit to cause preinstall to run the prepublish script if needed, and leave it alone if they don't.

dfellis commented Jan 15, 2013

I don't believe it's necessary. If the user has forked a project and is installing via a raw git repo, they could add a commit to cause preinstall to run the prepublish script if needed, and leave it alone if they don't.

@guybrush

This comment has been minimized.

Show comment
Hide comment
@guybrush

guybrush Jan 17, 2013

Contributor

I am with @dfellis after thinking 2 days about this and I am still not sure haha.

My thoughts are:

  • if a package is promoted to be installed via git, this would be unnecessary anyway. the developer will make sure it works.
  • if a package is actually in the registry but installed via git, the user knows what he is doing and will do as @dfellis mentioned (or any other solution)
  • the devDependencies-problem sucks, prepublish is meant to be run on the developer-side

But I think it should be mentioned in the docs maybe?

Contributor

guybrush commented Jan 17, 2013

I am with @dfellis after thinking 2 days about this and I am still not sure haha.

My thoughts are:

  • if a package is promoted to be installed via git, this would be unnecessary anyway. the developer will make sure it works.
  • if a package is actually in the registry but installed via git, the user knows what he is doing and will do as @dfellis mentioned (or any other solution)
  • the devDependencies-problem sucks, prepublish is meant to be run on the developer-side

But I think it should be mentioned in the docs maybe?

@iki

This comment has been minimized.

Show comment
Hide comment
@iki

iki Apr 29, 2013

use case:

  • condition: package needs to be compiled before installation, e.g. from coffeescript to javascript
  • condition: compiled files are not welcome in the repository
  • goal: allow developer to publish package in registry in a compiled state, so common users do not need any pre/postinstall
  • goal: allow users to install a different versions or forks of the package from git repos without having to do anything special (e.g. as a project dependencies)

sample current solution by @paulmillr at https://github.com/brunch/brunch:

  • on prepublish compile all and manually turn off postinstall in package,json
  • on postpublish reenable postinstall
  • on postinstall (which runs only when installed via git, as it is disabled in packages in registry): compile all

limitations:

  • clearly devDependencies will need to be installed for packages with prepublish installed via git repo ... this is ok as long as users did really need to run preinstall, in this case it is the right thing to do
  • complex prepublish scripts may depend on other development tools, that may not be available on user machines

options:

  1. don't do anything, let developers solve that like in https://github.com/brunch/brunch ... imho awkward
  2. run prepublish on git install ... will fail packages with complex prepublish scripts installed via git repo (but currently they were just installed unbuilt anyway) and force people to simple, or all-js scripts (may be both a good practice, and maybe too restrictive in some cases)
  3. add and run gitinstalll on installing via git repo, install gitDependencies in that case ... adds new settings, but breaks no compatibility and solves the issue, my 2c

iki commented Apr 29, 2013

use case:

  • condition: package needs to be compiled before installation, e.g. from coffeescript to javascript
  • condition: compiled files are not welcome in the repository
  • goal: allow developer to publish package in registry in a compiled state, so common users do not need any pre/postinstall
  • goal: allow users to install a different versions or forks of the package from git repos without having to do anything special (e.g. as a project dependencies)

sample current solution by @paulmillr at https://github.com/brunch/brunch:

  • on prepublish compile all and manually turn off postinstall in package,json
  • on postpublish reenable postinstall
  • on postinstall (which runs only when installed via git, as it is disabled in packages in registry): compile all

limitations:

  • clearly devDependencies will need to be installed for packages with prepublish installed via git repo ... this is ok as long as users did really need to run preinstall, in this case it is the right thing to do
  • complex prepublish scripts may depend on other development tools, that may not be available on user machines

options:

  1. don't do anything, let developers solve that like in https://github.com/brunch/brunch ... imho awkward
  2. run prepublish on git install ... will fail packages with complex prepublish scripts installed via git repo (but currently they were just installed unbuilt anyway) and force people to simple, or all-js scripts (may be both a good practice, and maybe too restrictive in some cases)
  3. add and run gitinstalll on installing via git repo, install gitDependencies in that case ... adds new settings, but breaks no compatibility and solves the issue, my 2c

@swang swang referenced this issue in clutchski/coffeelint May 12, 2013

Closed

Cannot npm install from git URL #99

@squaremo squaremo referenced this issue in squaremo/amqp.node Sep 1, 2013

Closed

Not found './defs' #13

@alliv8 alliv8 referenced this issue in shimaore/esl Oct 28, 2013

Closed

Cant fork successfully #10

@seishun

This comment has been minimized.

Show comment
Hide comment
@seishun

seishun Nov 15, 2013

I agree. npm install with no args already runs the prepublish script for the same reason, so it would make sense to do the same for git urls.

seishun commented Nov 15, 2013

I agree. npm install with no args already runs the prepublish script for the same reason, so it would make sense to do the same for git urls.

@mrjackdavis

This comment has been minimized.

Show comment
Hide comment
@mrjackdavis

mrjackdavis Feb 19, 2014

I also agree. Repositories are for the source files, why should we have to add extra bulk (especially for larger packages)?
Since the idea of version control is to control the source, npm should assume that only the source is contained in a git repo and run prepublish when installing a package from git. It is theoretically identical to installing a package in a local directory.

I also agree. Repositories are for the source files, why should we have to add extra bulk (especially for larger packages)?
Since the idea of version control is to control the source, npm should assume that only the source is contained in a git repo and run prepublish when installing a package from git. It is theoretically identical to installing a package in a local directory.

@kmees kmees referenced this issue in kmees/karma-sinon-chai Mar 17, 2014

Closed

Install bower on postinstall #5

joshprice added a commit to Tabcorp/stubby4node that referenced this issue Apr 23, 2014

Check in the compiled source
npm doesn't run the prepublish task to compile the coffee when
installing from github urls (but does locally).

See npm/npm#3055

@dantman dantman referenced this issue in postcss/postcss Jun 18, 2014

Closed

Cannot depend on postcss' git repo #52

alfred-landrum pushed a commit to jut-io/node-oniguruma that referenced this issue Jul 10, 2014

@cybertk

This comment has been minimized.

Show comment
Hide comment
@cybertk

cybertk Oct 6, 2014

Any updates?

cybertk commented Oct 6, 2014

Any updates?

@denis-sokolov

This comment has been minimized.

Show comment
Hide comment
@denis-sokolov

denis-sokolov Oct 6, 2014

npm has 861 open issues and is a bit behind at handling them. It will take a while to respond.

I personally would appreciate if I didn't get notifications about people inquiring about updates, but, alas.

npm has 861 open issues and is a bit behind at handling them. It will take a while to respond.

I personally would appreciate if I didn't get notifications about people inquiring about updates, but, alas.

natevw added a commit to natevw/node-hid that referenced this issue Oct 15, 2014

@natevw

This comment has been minimized.

Show comment
Hide comment
@natevw

natevw Oct 15, 2014

Yes, between this and #1876 I can't figure out any good way to use a forked module in dependent project, without publishing it to the public repo.

Backstory: I'm trying to get https://github.com/natevw/node-hid/tree/dhleong/Node-0.11.13 building for atom-shell on Windows. Somethings odd, and I'm trying to simplify how the build process works, but between these two issues there's not really a good way depend on git submodules in a non-published node module.

natevw commented Oct 15, 2014

Yes, between this and #1876 I can't figure out any good way to use a forked module in dependent project, without publishing it to the public repo.

Backstory: I'm trying to get https://github.com/natevw/node-hid/tree/dhleong/Node-0.11.13 building for atom-shell on Windows. Somethings odd, and I'm trying to simplify how the build process works, but between these two issues there's not really a good way depend on git submodules in a non-published node module.

@braco

This comment has been minimized.

Show comment
Hide comment
@braco

braco Dec 2, 2014

@natevw: try npm shrinkwrap, which is infuriating to use in the longer term, but can help with what you're describing.

braco commented Dec 2, 2014

@natevw: try npm shrinkwrap, which is infuriating to use in the longer term, but can help with what you're describing.

@natevw

This comment has been minimized.

Show comment
Hide comment
@natevw

natevw Dec 4, 2014

@braco Thanks for documenting that, it is similar to what I ended up doing (which was to make a GitHub release tarball and install that instead).

natevw commented Dec 4, 2014

@braco Thanks for documenting that, it is similar to what I ended up doing (which was to make a GitHub release tarball and install that instead).

@john-kurkowski

This comment has been minimized.

Show comment
Hide comment
@john-kurkowski

john-kurkowski Jan 16, 2015

By the time a Git URL's source code makes it to my node_modules, the repo's .npmignore has been applied.

the developer will make sure it works.

I don't get the full source from which to build. So I can't make sure it works. Not within package.json, anyway.

By the time a Git URL's source code makes it to my node_modules, the repo's .npmignore has been applied.

the developer will make sure it works.

I don't get the full source from which to build. So I can't make sure it works. Not within package.json, anyway.

ursm added a commit to idobata/idobata-hooks that referenced this issue Jan 24, 2015

ursm added a commit to idobata/idobata-hooks that referenced this issue Jan 24, 2015

ursm added a commit to idobata/idobata-hooks that referenced this issue Mar 4, 2015

@roblabla roblabla referenced this issue in PrismarineJS/node-minecraft-protocol Mar 8, 2015

Closed

Error: Cannot find module './dist/index.js' #125

natevw added a commit to natevw/node-hid that referenced this issue Apr 3, 2015

@fmoo fmoo referenced this issue in fmoo/react-typeahead Apr 9, 2015

Merged

Added gulp build task for webpack support #49

ianobermiller added a commit to FormidableLabs/radium that referenced this issue May 16, 2015

Don't exclude src from npm
If we do, and you install via a git url in package.json, you won’t be
able to use it at all. See
npm/npm#3055 (comment) for a
potential workaround.
@BerkeleyTrue

This comment has been minimized.

Show comment
Hide comment
@BerkeleyTrue

BerkeleyTrue Jul 7, 2015

Any word on this?

Any word on this?

felixrabe added a commit to felixrabe/rollup-plugin-commonjs that referenced this issue Oct 2, 2017

paradoxxxzero added a commit to paradoxxxzero/react-draft-wysiwyg that referenced this issue Oct 10, 2017

@max-b max-b referenced this issue in goldhand/sw-precache-webpack-plugin Oct 10, 2017

Merged

Replacing prepublish script with prepare. #112

@JeremyPlease JeremyPlease referenced this issue in prescottprue/react-redux-firebase Oct 18, 2017

Merged

v2.0.0-beta.11 #295

2 of 3 tasks complete
@m0v0nage

This comment has been minimized.

Show comment
Hide comment
@m0v0nage

m0v0nage Oct 27, 2017

If a project is going to be able to build an npm package from source, wouldn't it always need to install devDependencies (some) and peerDependencies for that package. Doesn't look like npm does that currently.

If a project is going to be able to build an npm package from source, wouldn't it always need to install devDependencies (some) and peerDependencies for that package. Doesn't look like npm does that currently.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Oct 27, 2017

If NPM isn't, then it definitely should.

trusktr commented Oct 27, 2017

If NPM isn't, then it definitely should.

@AlexanderOMara

This comment has been minimized.

Show comment
Hide comment
@AlexanderOMara

AlexanderOMara Oct 29, 2017

@erikfox @mturley @zkat

prepare will run if you install a git dependency that has a prepare script. [...]

This doesn't appear to be the case for me:

Same here. prepare only runs before packing and publishing, and when you npm install inside the package itself. It does not run for dependencies, only preinstall, install, & postinstall do.

Here is a package you can use to test this and see what scripts get run: https://github.com/AlexanderOMara/test-package-json-scripts

I don't think the problem described in this issue is actually solved, although it was closed.

AlexanderOMara commented Oct 29, 2017

@erikfox @mturley @zkat

prepare will run if you install a git dependency that has a prepare script. [...]

This doesn't appear to be the case for me:

Same here. prepare only runs before packing and publishing, and when you npm install inside the package itself. It does not run for dependencies, only preinstall, install, & postinstall do.

Here is a package you can use to test this and see what scripts get run: https://github.com/AlexanderOMara/test-package-json-scripts

I don't think the problem described in this issue is actually solved, although it was closed.

@kafkahw

This comment has been minimized.

Show comment
Hide comment
@kafkahw

kafkahw Nov 10, 2017

@AlexanderOMara We're facing the same issue here.
To use, prepare runs with npm install only on Mac OS 10.12.6, but not on Red Hat Linux or Ubuntu/Debian. Any thoughts?
EDIT: we're on npm5 + node8

kafkahw commented Nov 10, 2017

@AlexanderOMara We're facing the same issue here.
To use, prepare runs with npm install only on Mac OS 10.12.6, but not on Red Hat Linux or Ubuntu/Debian. Any thoughts?
EDIT: we're on npm5 + node8

@AlexanderOMara

This comment has been minimized.

Show comment
Hide comment
@AlexanderOMara

AlexanderOMara Nov 10, 2017

@kafkahw AFAIK, prepare script are not supposed to run for dependencies at all, on any platform (I tested on macOS 10.12.something). I don't know if/how you got a different result.

AlexanderOMara commented Nov 10, 2017

@kafkahw AFAIK, prepare script are not supposed to run for dependencies at all, on any platform (I tested on macOS 10.12.something). I don't know if/how you got a different result.

@kafkahw

This comment has been minimized.

Show comment
Hide comment
@kafkahw

kafkahw Nov 13, 2017

@AlexanderOMara I created a test repo that demonstrates npm install might trigger dependency's prepare script. Feel free to check it out here: https://github.com/kafkahw/npm-install-test.

Basically, npm-install-test has a dependency called node-package1 that has a prepare script calling a missing method. When I npm install package npm-install-test, I got an error about that missing method from prepare script of dependency node-package1.

I tested this on Mac OS 10.12.6

kafkahw commented Nov 13, 2017

@AlexanderOMara I created a test repo that demonstrates npm install might trigger dependency's prepare script. Feel free to check it out here: https://github.com/kafkahw/npm-install-test.

Basically, npm-install-test has a dependency called node-package1 that has a prepare script calling a missing method. When I npm install package npm-install-test, I got an error about that missing method from prepare script of dependency node-package1.

I tested this on Mac OS 10.12.6

@AlexanderOMara

This comment has been minimized.

Show comment
Hide comment
@AlexanderOMara

AlexanderOMara Nov 14, 2017

@kafkahw Well, now I'm even more confused... I too get the prepare script failed issue using your repo. Beyond that though, I can't make sense of what is going on.

If I npm install on this package:

{
  "name": "test-module-git",
  "dependencies": {
    "test-package-json-scripts": "git+https://github.com/AlexanderOMara/test-package-json-scripts.git"
  }
}

Then node_modules/test-package-json-scripts/log.txt looks like this:

2017-11-14T00:29:50.592Z: preinstall
2017-11-14T00:29:50.698Z: install
2017-11-14T00:29:50.798Z: postinstall

Leaving no sign that prepare ran on the dependency (I'm not sure if it actually ran or not).

If I use yarn install instead though, I get a node_modules/test-package-json-scripts/log.txt that shows prepare was run:

2017-11-14T00:19:00.952Z: preinstall
2017-11-14T00:19:01.076Z: install
2017-11-14T00:19:01.185Z: postinstall
2017-11-14T00:19:01.292Z: prepare
2017-11-14T00:27:09.047Z: preinstall
2017-11-14T00:27:09.156Z: install
2017-11-14T00:27:09.258Z: postinstall

Tested using macOS 10.12.6, npm 5.4.2, yarn 1.3.2.

At this point, I'm really confused as to how this is suppose to work and why Yarn seems to work in my case, but NPM does not.

@kafkahw Well, now I'm even more confused... I too get the prepare script failed issue using your repo. Beyond that though, I can't make sense of what is going on.

If I npm install on this package:

{
  "name": "test-module-git",
  "dependencies": {
    "test-package-json-scripts": "git+https://github.com/AlexanderOMara/test-package-json-scripts.git"
  }
}

Then node_modules/test-package-json-scripts/log.txt looks like this:

2017-11-14T00:29:50.592Z: preinstall
2017-11-14T00:29:50.698Z: install
2017-11-14T00:29:50.798Z: postinstall

Leaving no sign that prepare ran on the dependency (I'm not sure if it actually ran or not).

If I use yarn install instead though, I get a node_modules/test-package-json-scripts/log.txt that shows prepare was run:

2017-11-14T00:19:00.952Z: preinstall
2017-11-14T00:19:01.076Z: install
2017-11-14T00:19:01.185Z: postinstall
2017-11-14T00:19:01.292Z: prepare
2017-11-14T00:27:09.047Z: preinstall
2017-11-14T00:27:09.156Z: install
2017-11-14T00:27:09.258Z: postinstall

Tested using macOS 10.12.6, npm 5.4.2, yarn 1.3.2.

At this point, I'm really confused as to how this is suppose to work and why Yarn seems to work in my case, but NPM does not.

@kafkahw

This comment has been minimized.

Show comment
Hide comment
@kafkahw

kafkahw Nov 14, 2017

@AlexanderOMara I logged an issue #19152 to track this problem.

kafkahw commented Nov 14, 2017

@AlexanderOMara I logged an issue #19152 to track this problem.

jodigiordano added a commit to metriodev/react-handsontable that referenced this issue Nov 16, 2017

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Dec 28, 2017

prepare seems to be working for me now, packages from git are built properly, at least the ones I just tried. Maybe it's been fixed in newer version of npm?

P.S., first time ever installing a package on a machine takes a while; I'm guessing it installs dev dependencies because that's the only way it can possibly build the package. The second time installing the package is fast, from the cache in ~/.npm.

trusktr commented Dec 28, 2017

prepare seems to be working for me now, packages from git are built properly, at least the ones I just tried. Maybe it's been fixed in newer version of npm?

P.S., first time ever installing a package on a machine takes a while; I'm guessing it installs dev dependencies because that's the only way it can possibly build the package. The second time installing the package is fast, from the cache in ~/.npm.

@v1adko

This comment has been minimized.

Show comment
Hide comment
@v1adko

v1adko Dec 28, 2017

@erikfox I'm facing the same issue. Need to transpile code after fetching it from a private git repo.
Build script works correctly on the repo directly, but not when it gets installed into node_modules.
Only one file in the dist not the whole src directory file structure.

Were you able to resolve this issue? Not sure if it's an npm's problem or babel's.

v1adko commented Dec 28, 2017

@erikfox I'm facing the same issue. Need to transpile code after fetching it from a private git repo.
Build script works correctly on the repo directly, but not when it gets installed into node_modules.
Only one file in the dist not the whole src directory file structure.

Were you able to resolve this issue? Not sure if it's an npm's problem or babel's.

@v1adko

This comment has been minimized.

Show comment
Hide comment
@v1adko

v1adko Dec 28, 2017

Allright. Got it to work. "files" in package.json and a non-obvious npm behavior regarding .npmignore and defaulting to .gitignore were the key for me.

Basically if you don't have .npmignore it uses .gitignore in its place and usually you don't want to commit your dist and have it in .gitignore...

Adding "dist" to files in package overwrites all ignores and does not cleanup after the build.

Here are the docs for all of the future adventurers.

v1adko commented Dec 28, 2017

Allright. Got it to work. "files" in package.json and a non-obvious npm behavior regarding .npmignore and defaulting to .gitignore were the key for me.

Basically if you don't have .npmignore it uses .gitignore in its place and usually you don't want to commit your dist and have it in .gitignore...

Adding "dist" to files in package overwrites all ignores and does not cleanup after the build.

Here are the docs for all of the future adventurers.

gre added a commit to LedgerHQ/ledgerjs that referenced this issue Jan 3, 2018

NoxHarmonium added a commit to agiledigital/react-dates that referenced this issue Jan 15, 2018

Add ‘prepare’ statement to package.json
This will run when installing ‘react-dates’ directly from the git repo so that build artifacts are available. This is useful when testing.

See:
- npm/npm#3055 (comment)
@shellscape

This comment has been minimized.

Show comment
Hide comment
@shellscape

shellscape Jan 15, 2018

FWIW prepare doesn't appear to execute when installing a git dependency from a branch on npm@latest

shellscape commented Jan 15, 2018

FWIW prepare doesn't appear to execute when installing a git dependency from a branch on npm@latest

@strugee

This comment has been minimized.

Show comment
Hide comment
@strugee

strugee Jan 16, 2018

Contributor

@shellscape file a new bug and provide detailed environment details/steps to reproduce

Contributor

strugee commented Jan 16, 2018

@shellscape file a new bug and provide detailed environment details/steps to reproduce

jupereira0920 added a commit to jupereira0920/angular2-qrscanner that referenced this issue Jan 23, 2018

add prepare to npm scripts
npm@5.0.0 now uses prepare instead of prepublish
Related issue npm/npm#3055

I keep both, to test first.

manuelvillar added a commit to manuelvillar/react-remarkable that referenced this issue Feb 23, 2018

manuelvillar added a commit to manuelvillar/react-remarkable that referenced this issue Feb 23, 2018

@brunocodutra brunocodutra referenced this issue in brunocodutra/webapp-webpack-plugin Mar 22, 2018

Merged

Use `prepare` #56

mcmillhj-wta added a commit to willowtreeapps/react-digraph that referenced this issue Apr 4, 2018

add preinstall step in order to run prepublish
When installing with NPM via a github URL, the prepublish hook is not run: npm/npm#3055. The workaround for this is to add a preinstall step that in turn calls the prepublish step.

@mcmillhj-wta mcmillhj-wta referenced this issue in willowtreeapps/react-digraph Apr 4, 2018

Merged

add preinstall step in order to run prepublish #1

ltetzlaff added a commit to ltetzlaff/pdf.js that referenced this issue Apr 24, 2018

ltetzlaff added a commit to ltetzlaff/pdf.js that referenced this issue Apr 24, 2018

@NotAmaan NotAmaan referenced this issue in mcchrish/feathers-objection May 15, 2018

Merged

Added prepare script #15

vith added a commit to vith/uppy that referenced this issue May 22, 2018

Npm scripts: use "prepare" instead of "prepublishOnly"
When a prepare script is defined, npm and yarn will build uppy
automatically if it's being depended on through a git URL.
This is useful when downstream projects don't want to wait for
the next release to be tagged.

See npm/npm#3055 (comment)
See also yarnpkg/yarn#3553

thinkh added a commit to Caleydo/lineupjs that referenced this issue Jun 12, 2018

Add npm prepare script
Adding the `prepare` script allows to install this repo from git uri:
`npm install --save git+https://github.com/datavisyn/lineupjs.git`

For more information see:

* https://docs.npmjs.com/misc/scripts
* npm/npm#3055
* Example: tasti/react-linkify#61

@thinkh thinkh referenced this issue in datavisyn/lineupjs Jun 12, 2018

Closed

Add npm prepare script to allow install via git URI #20

3 of 3 tasks complete

thinkh added a commit to thinkh/lineupjs that referenced this issue Jun 13, 2018

Replace npm script `prepublishOnly` with `prepare`
Adding the `prepare` script allows to install this repo from git uri:
`npm install --save git+https://github.com/datavisyn/lineupjs.git`

For more information see:

* docs.npmjs.com/misc/scripts
* npm/npm#3055

@thinkh thinkh referenced this issue in datavisyn/lineupjs Jun 13, 2018

Merged

Replace npm script `prepublishOnly` with `prepare` #24

3 of 3 tasks complete
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.