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

[BUG] prepare and prepack scripts are not handled as documented when installing a git dependency #1865

Closed
kimamula opened this issue Sep 27, 2020 · 25 comments
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release

Comments

@kimamula
Copy link

kimamula commented Sep 27, 2020

Current Behavior:

I tested installing a git dependency which has either prepare or prepack script with npm CLI v7.0.0-beta.12 and confirmed the following behaviors.

  • a package with a prepare script: devDependencies are NOT installed but the prepare script is executed
  • a package with a prepack script: devDependencies are NOT installed and the prepack script is NOT executed

Expected Behavior:

  • a package with a prepare script: devDependencies are installed and the prepare script is executed
  • a package with a prepack script: devDependencies are installed and the prepack script is executed
    • Though I've not found any document saying that devDependencies are installed when installing a git dependency with a prepack script and actually they are not installed even when npm CLI v6 is used, I believe npm should install them since running a prepack script without installing devDependencies does not make sense.

Steps To Reproduce:

prepare:

// https://github.com/kimamula/prepare-test/blob/master/package.json
{
  "name": "prepare-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "prepare": "tsc"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "typescript": "^4.0.3"
  }
}
$ npm install github:kimamula/prepare-test
npm ERR! code 127
npm ERR! git dep preparation failed
npm ERR! command /Users/kenji/.nodenv/versions/12.18.3/bin/node /Users/kenji/.nodenv/versions/12.18.3/lib/node_modules/npm/bin/npm-cli.js install --only=dev --prod --ignore-prepublish --no-progress --no-save --cache=/Users/kenji/.npm/_cacache --prefer-offline=false --prefer-online=false --offline=false --before=
npm ERR! > [email protected] prepare
npm ERR! > tsc
npm ERR! npm WARN invalid config before="" set in command line options
npm ERR! npm WARN invalid config Must be one of: null, valid Date string
npm ERR! sh: tsc: command not found
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /Users/kenji/.npm/_cacache/tmp/git-clone-a73f300b
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c tsc
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in:
npm ERR! npm ERR!     /Users/kenji/.npm/_cacache/_logs/2020-09-27T06_13_19_941Z-debug.log

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kenji/.npm/_logs/2020-09-27T06_13_19_972Z-debug.log

prepack:

// https://github.com/kimamula/prepack-test/blob/master/package.json
{
  "name": "prepack-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "prepack": "tsc"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "typescript": "^4.0.3"
  }
}
$ npm install github:kimamula/prepack-test

changed 1 package, and audited 1 package in 6s

found 0 vulnerabilities

-> tsc should generate index.js file, but it's not generated.

Environment:

  • OS: macOS 10.15.6
  • Node: 12.18.3
  • npm: 7.0.0-beta.12
@kimamula kimamula added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Sep 27, 2020
@darcyclarke darcyclarke added beta and removed Needs Triage needs review for next steps labels Oct 1, 2020
@darcyclarke darcyclarke added this to the OSS - Sprint 16 milestone Oct 1, 2020
@darcyclarke
Copy link
Contributor

@kimamula just to confirm, does this behavior reflect how [email protected] works today? Or, have we changed how that works?

@kimamula
Copy link
Author

kimamula commented Oct 2, 2020

@darcyclarke The behavior of [email protected] is different (see #1229).

  • a package with a prepare script: working as expected (devDependencies are installed and the prepare script is executed)
  • a package with a prepack script: devDependencies are NOT installed but the prepack script is executed

@aomarks
Copy link

aomarks commented Oct 21, 2020

I seem to be hitting a similar case but where the prepare script is running without dependencies being available under NPM 7. This particular dependency is not published to NPM, so I am installing it as a Git dependency specification, not sure if that is related.

With NPM 6, the following worked:

npm install --save-dev git+https://github.com/codemirror/google-modes.git#57b26bb0e76ca5d3b83b12faf13ce1054d34bdd

But with NPM 7 it fails, seemingly because the grammar-mode binary that this package's prepare script depends on is not available:

npm install --save-dev git+https://github.com/codemirror/google-modes.git#57b26bb0e76ca5d3b83b12faf13ce1054d34bdd
npm ERR! code 127
npm ERR! git dep preparation failed
npm ERR! command /home/aomarks/node/bin/node /home/aomarks/node/lib/node_modules/npm/bin/npm-cli.js install --only=dev --prod --ignore-prepublish --no-progress --no-save --cache=/home/aomarks/.npm/_cacache --prefer-offline=false --prefer-online=false --offline=false --before=
npm ERR! > [email protected] prepare
npm ERR! > npm run build && npm test
npm ERR! 
npm ERR! 
npm ERR! > [email protected] build
npm ERR! > npm run build-js && npm run build-ts && npm run build-c && npm run build-cpp && npm run build-clif && npm run build-fbs && npm run build-py && npm run build-go && npm run build-java && npm run build-html && npm run build-ng && npm run build-kotlin
npm ERR! 
npm ERR! 
npm ERR! > [email protected] build-js
npm ERR! > grammar-mode --es-module src/javascript.grammar --output src/javascript.mode.js && rollup -c rollup.config.js src/javascript.js -o dist/javascript.js
npm ERR! npm WARN invalid config before="" set in command line options
npm ERR! npm WARN invalid config Must be one of: null, valid Date string
npm ERR! sh: 1: grammar-mode: not found
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /home/aomarks/.npm/_cacache/tmp/git-clone-ae2eb4f5
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c grammar-mode --es-module src/javascript.grammar --output src/javascript.mode.js && rollup -c rollup.config.js src/javascript.js -o dist/javascript.js
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in:
npm ERR! npm ERR!     /home/aomarks/.npm/_cacache/_logs/2020-10-21T22_12_07_990Z-debug.log
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /home/aomarks/.npm/_cacache/tmp/git-clone-ae2eb4f5
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c npm run build-js && npm run build-ts && npm run build-c && npm run build-cpp && npm run build-clif && npm run build-fbs && npm run build-py && npm run build-go && npm run build-java && npm run build-html && npm run build-ng && npm run build-kotlin
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in:
npm ERR! npm ERR!     /home/aomarks/.npm/_cacache/_logs/2020-10-21T22_12_08_013Z-debug.log
npm ERR! npm ERR! code 127
npm ERR! npm ERR! path /home/aomarks/.npm/_cacache/tmp/git-clone-ae2eb4f5
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c npm run build && npm test
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in:
npm ERR! npm ERR!     /home/aomarks/.npm/_cacache/_logs/2020-10-21T22_12_08_023Z-debug.log

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/aomarks/.npm/_logs/2020-10-21T22_12_08_070Z-debug.log

zloirock added a commit to zloirock/core-js that referenced this issue Oct 26, 2020
@zloirock
Copy link

This bug causes problems for core-js.

https://travis-ci.org/github/zloirock/core-js/builds/738834230

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2020

This is a bug in pacote. npm/pacote#53

@isaacs
Copy link
Contributor

isaacs commented Nov 5, 2020

Will be fixed in the next cli release (ie, tomorrow, likely included with the Node.js 15 release going out next week).

@isaacs
Copy link
Contributor

isaacs commented Nov 6, 2020

Yeah, those look like duplicates.

@fregante
Copy link
Contributor

fregante commented Dec 30, 2020

Can anyone double-check that this problem has actually been fixed? prepack is not being run in npm 7.0.9 nor the latest npm (7.3.0). prepare works.

npm i fregante/select-dom#edd18f4 --verbose # with prepack
npm i fregante/select-dom#32f1864 --verbose # with prepare

Testing on Node v15.2.1, macOS 10.15.7. I tried with npm 6 and prepack is being run (but it fails, likely due to missing dev dependencies)

@aomarks
Copy link

aomarks commented Mar 3, 2021

I can confirm it works for me now, at least for my particular case outlined here: #1865 (comment)

@fregante
Copy link
Contributor

fregante commented Mar 3, 2021

Still no luck for me on npm 7.6.0. This still results in an semi-empty package folder

npm i fregante/select-dom#edd18f4 --verbose # with prepack

@zimtsui
Copy link

zimtsui commented Jul 12, 2021

why is this issue closed? the problem has not been solved in 7.19.1

kyliau pushed a commit to kyliau/lighthouse-stack-packs that referenced this issue Nov 16, 2021
During npm install, the Lighthouse package which is installed from GitHub
(@master branch) triggers the newly added `prepack` script [1], but due to
a bug in NPM [2] the devDependencies are not available when the script
is run, thus making the installation fail.

[1]: GoogleChrome/lighthouse#13261
[2]: npm/cli#1865
kyliau pushed a commit to kyliau/lighthouse-stack-packs that referenced this issue Nov 16, 2021
During npm install, the Lighthouse package which is installed from GitHub
(@master branch) triggers the newly added `prepack` script [1], but due to
a bug in NPM [2] the devDependencies are not available when the script
is run, thus making the installation fail.

[1]: GoogleChrome/lighthouse#13261
[2]: npm/cli#1865
@jameskerr
Copy link

There were two problems documented in this issue.

  1. dev dependencies were not being installed
  2. prepack was not running

The first has been fixed, the second has not.

Repro Example

I have a test package called npm-lib with the following package.json

{
  "scripts": {
    "prepack": "ls | cowsay > prepacked.txt",
    "prepare": "ls | cowsay > prepared.txt"
  },
  "devDependencies": {
    "cowsay": "^1.5.0"
  }
}

I add this to another package like so with the commit hash:

"dependencies": {
    "npm-lib": "github:jameskerr/npm-lib#7cb685db9a232c44057b2be3a97cff183972b4cd"
  },

I then run npm install with npm@6 and npm@7 and look at the contents of node_modules/npm-lib.

npm version 6.14.15

$ ls node_modules/npm-lib
README.md     
index.js      
package.json  
prepacked.txt 
prepared.txt

npm version 7.24.2

ls node_modules/npm-lib
README.md    
index.js     
package.json 
prepared.txt

The contents of the prepared.txt show that the dev dependency (cowsay) was installed:

cat node_modules/npm-lib/prepared.txt
 ___________________
/ README.md         \
| index.js          |
| main.go           |
| node_modules      |
| package-lock.json |
| package.json      |
\ prepared.txt      /
 -------------------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

Should I open a new issue?

@fwcd
Copy link

fwcd commented Jan 12, 2023

I can confirm that the issue that devDependencies are not installed before prepare is executed is not solved, I have a TypeScript package with a package.json like this:

{
  "scripts": {
    "prepare": "tsc ..."
  },
  "devDependencies": {
    "typescript": "..."
  }
}

Running npm install with npm 9.2.0 errors with

npm ERR! sh: tsc: command not found

whereas yarn handles this installation perfectly well. As a temporary workaround the required devDependencies can be installed globally, in this case the TypeScript compiler, but as a solution this isn't great.

fwcd added a commit to ProjectLighthouseCAU/nighthouse that referenced this issue Jan 12, 2023
This still seems to cause trouble though, see
npm/cli#1865
@aarondill
Copy link

I can confirm that on npm version 9.4.1, prepack is still not being called when installing git dependencies.
the test in #1865 (comment) demonstrate this issue perfectly. prepare is run, but prepack is never called.

@aarondill
Copy link

aarondill commented Feb 4, 2023

workspaces/libnpmpack/lib/index.js is the only place in this codebase which calls prepack, it seems like the dependency pacote anticipates the cli to handle prepack(pacote/blob/main/lib/dir.js#L39-L41), yet it's only handled on npm pack and npm publish
A new issue in npm/pacote has been created to correspond with this one until the problem is solved: npm/pacote#257

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2023

Why would prepack run with git deps, when they’re never packed or published?

@fregante
Copy link
Contributor

fregante commented Feb 4, 2023

That makes sense, but the lifecycle of npm scripts still seems alien to me. I recall prepack shortly being the preferred way to execute scripts before publishing a few years back.

Installing from git bypasses the entire packaging and publishing sequence that repos go through when running npm publish, in a way I'd expect the same sequence to be repeated when running npm install user/repo. Otherwise we get these weird lifecycle events that vary depending on how the package is installed.

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2023

Prepack works great for a “before publish” step; i use it - but git deps are never published. prepare is used when you want to support installing from git (which i have no interest in), and it should run pre publish and pre install.

@fregante
Copy link
Contributor

fregante commented Feb 4, 2023

Prepack works great for a “before publish” step; i use it -

Exactly, but why do you use it? Is it to run a build? Then it'd make sense to have a script that runs when installing from git

to support installing from git (which i have no interest in)

Do you say that as a module author? Then yeah, that's not great, is it. As a module author myself, I shouldn't need to bother about that either.

Does npm install support installing from git or not? These half-implementations really cause a lot of confusion. Why should anyone use prepack if prepare is the right one to support both? I wish there was one way to do things in npm land.

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2023

I use it to autogenerate an npmignore file, which isn’t a relevant task unless it’s being published.

As a module author, i always need to bother with the way my users install things - anything i support is something i have to support, and since i believe nothing should ever be consumed except from an npm registry, that’s what i wish to support.

npm install supports that, but the thing you’re installing may not.

@aarondill
Copy link

aarondill commented Feb 5, 2023

Why should anyone use prepack if prepare is the right one to support both?

prepare runs on local npm install with no arguments, thus when cloning a repo and installing dependencies, the prepare script is run, which would likely build the project, as well as filling the terminal with lots of information that's only relevant to building(as many build steps output debug information)

From the docs, prepack should run everywhere prepare does, except for local npm install, thus still allowing usage of npm pack and installation from git repos, without auto-building when installing dependencies.

@jakebailey
Copy link

Not to pile on here, but, we were actually considering adding a prepack to TypeScript to make bisecting checker changes simpler, only to find out that prepack doesn't actually work in v7+ anymore.

prepare is the wrong script to be using for our needs, as we'd incur a full build at a plain npm install or even npm ci at the repo root, wheras prepack is much better and used by yarn. But, given npm doesn't seem to support it anymore, we just gave up on it.

Does this need to be filed as a brand new issue? Reopened? Is npm/pacote#257 the right place for this now?

@aovchinn
Copy link

aovchinn commented May 9, 2023

is this happening again ?
I tested it on npm 7 and 9
created #6435

@jakebailey
Copy link

jakebailey commented May 9, 2023

@aovchinn Not sure your linked issue is relavent to this issue; this issue is about npm not executing the prepack script on installs via git in v7+, not about missing things on PATH.

Er, no, sorry. This issue contains multiple issues all in one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests