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

Installing package from github URL doesn't trigger prepublish script #2875

Closed
OmgImAlexis opened this issue Mar 9, 2017 · 5 comments · Fixed by #3553
Closed

Installing package from github URL doesn't trigger prepublish script #2875

OmgImAlexis opened this issue Mar 9, 2017 · 5 comments · Fixed by #3553
Labels

Comments

@OmgImAlexis
Copy link

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

What is the current behavior?
Installing package from Github URL doesn't run the prepublish script

What is the expected behavior?
Yarn should run the prepublish script when installing from a Github URL just like it should when installing a local package.

Ref: npm/npm#3055
More specifically: npm/npm#3055 (comment)

@itsnotvalid
Copy link

Is there any updates?

@stereobooster
Copy link

Technically speaking this is feature, not a bug, because this is not implemented in npm either.

I would expect following: when dependency installed from git and there is prepublish hook, install dev dependencies for this repo and run hook. If there is no prepublish hook, no need to install dev dependencies, just git clone.

fcsonline added a commit to redbooth/babel-plugin-react-element-info that referenced this issue May 10, 2017
fcsonline added a commit to redbooth/babel-plugin-react-element-info that referenced this issue May 10, 2017
fcsonline added a commit to redbooth/babel-plugin-react-element-info that referenced this issue May 16, 2017
@Volune
Copy link
Contributor

Volune commented May 27, 2017

There was a suggestion on npm from @gagern:

How about this: clone the repo to a temporary directory, run npm install && npm pack there (which means pulling all dev dependencies), then untar the package tarball we just generated to the node_modules directory in question, and fetch (or perhaps move) any non-dev dependencies it has.

The repository is treated as a source repository, which means you'll have to assume that all dev dependencies and all prepublish scripts might be needed. For every point and purpose, building from that temporary directory is just like what you'd do prior to publishing, which should work if the package is sufficiently portable.
The permanently installed part is like a published package, though, with everything those have, but without anything .npmignore got rid of, and without unneccessary dev dependencies. For every point and purpose, the installed package behaves just like what you'd get from the registry if it were installed there.

It was not implemented because the npm code was too complex at that time.

I think this behavior can be implemented in Yarn, either as the default behavior, or with an option somewhere.
Regarding all the discussions about this issue on both npm and yarn, I think it should be the default behavior.

I'll have a look to prepare a PR for that.

@stereobooster
Copy link

Update:

@zkat commented 6 hours ago
This is now available in [email protected]. instead of prepublish, you'll need to use prepare, but this is otherwise as you'd expect. 🎉

@zkat
Copy link
Contributor

zkat commented May 30, 2017

Please let me know if y'all have any questions about how this works in npm now, and please note that it's fairly fresh and it's bound to have some kind of crap in there that I didn't realize would be a problem (and thus subject to change -- so if you find something, please let us know!)

The code in question npm-side is: https://github.com/npm/npm/blob/latest/lib/pack.js#L151-L221, which isn't exactly the simplest thing in the world. It essentially runs npm install in the project directory before packing it, preventing prepublish from executing, but only if a prepare script exists.

There's another backflip that happens where this function is actually passed to pacote, which invokes it when installing git dependencies. This was mainly so pacote didn't have to be aware of lifecycle scripts, and I assume will be simplified in the future (or have more of this be integrated into pacote proper).

wearhere added a commit to mixmaxhq/custody that referenced this issue May 8, 2018
We switch from yarn to npm because we’re installing `node-tail` from GitHub
(in order to pick up lucagrulla/node-tail#63 and
lucagrulla/node-tail#64), and this means that we need
to build it on installation (via the “prepare” script that
lucagrulla/node-tail#64 added), and yarn 0.24.6 doesn’t
run “prepare” scripts when installing from GitHub:
yarnpkg/yarn#2875.

That’s fixed in a more recent release of yarn but given that we’re about to
switch back to npm anyway, npm is simpler.

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

Successfully merging a pull request may close this issue.

7 participants