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

fix(npm packaging): restore prepare script in package.json so that npm install from github works #367

Merged
merged 2 commits into from
Nov 24, 2018

Conversation

appzuka
Copy link
Contributor

@appzuka appzuka commented Oct 19, 2018

Date: Fri Oct 19 16:08:37 2018 +0100

fix(npm packaging): restore prepare script in package.json so that npm install from github works

In commit 2a4d3b the prepare script was removed from package.json.  After that, it is no longer
possible to specify a github location in the project's package.json file to load a specific version
of the package from Github.  It should be possible to load the package using a command such as npm
install github:webpack-contrib/karma-webpack

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Breaking Changes

Additional Info

Date:   Fri Oct 19 16:08:37 2018 +0100

    fix(npm packaging): restore prepare script in package.json so that npm install from github works

    In commit 2a4d3b the prepare script was removed from package.json.  After that, it is no longer
    possible to specify a github location in the project's package.json file to load a specific version
    of the package from Github.  It should be possible to load the package using a command such as npm
    install github:webpack-contrib/karma-webpack
@jsf-clabot
Copy link

jsf-clabot commented Oct 19, 2018

CLA assistant check
All committers have signed the CLA.

@matthieu-foucault matthieu-foucault changed the title Author: Nick Excell <nick@appzuka.com> fix(npm packaging): restore prepare script in package.json so that npm install from github works Nov 23, 2018
@matthieu-foucault
Copy link
Collaborator

According to https://docs.npmjs.com/misc/scripts, prepare and prepublish are both executed when npm install is executed. How does that PR work, given that there is already a prepublish command?

@appzuka
Copy link
Contributor Author

appzuka commented Nov 23, 2018

It looks as if prepublish has been deprecated, although it is not very clear from the help. If I go to a clean directory, npm init, then type

npm install git+https://github.com/webpack-contrib/karma-webpack.git

Then no code is installed under node_modules/karma-webpack. If instead I type

npm install git+https://github.com/appzuka/karma-webpack.git

The package and its dependencies are installed into node_modules, as expected. I am using npm 6.4.1.

npm/npm#10074 gives a detailed description of the confusion. It is possible that users will see a different behaviour depending on the version of npm they are using.

I guess if you publish the package and users install from npmjs they will not see any issue. This is only an issue if users install directly from github. As described in https://docs.npmjs.com/cli/install the prepare script is run in this case so if there is no prepare script the package is not installed.

Now I wonder if having the build step in both prepare and prepublish will mess with the publishing process. It seems to me that it should just be in prepare, so that both publishing and installing from Github will work.

Copy link
Collaborator

@matthieu-foucault matthieu-foucault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that prepare fixes it. The prepublish script should be removed though, as it will trigger build twice when running npm install in the karma-webpack directory.

@appzuka
Copy link
Contributor Author

appzuka commented Nov 24, 2018

Agreed. I have removed the prepublish script and tested that installation works with npm directly from github and also with npm install in the local folder.

@matthieu-foucault matthieu-foucault merged commit 3e1f3e4 into codymikol:master Nov 24, 2018
trcarden pushed a commit to mancrates/karma-webpack that referenced this pull request Mar 4, 2020
- Prepare is run when yarn runs installs
- Prepare calls npm which isn't available on all machines
- We rename prepare to prepublihs like it was a few years ago
- Additional detail here:
  codymikol#367
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants