postpack script runs before the tgz is packed

cli
help-wanted
good-first-patch
priority:medium
triaged

(David Piepgrass) #1

What I Wanted to Do

I wanted to run a test procedure on the tarball produced by npm pack in my postpack script.

What Happened Instead

Contrary to intuition and the documentation of postpack, the tgz file does not exist within postpack.

Reproduction Steps

  • Run these teminal commands:

    mkdir temp
    cd temp
    npm init -f
    
  • Edit package.json to add a postpack script that accesses the tgz file:

    "postpack": "copy temp-1.0.0.tgz copy.tgz"
    
  • Run npm pack. The postpack command will fail because the tarball was not created. (It will, however, succeed if the postpack script is removed)

Platform Info

$ npm --versions
{ 'npm-testpack': '1.0.0',
  npm: '6.1.0',
  ares: '1.14.0',
  cldr: '33.0',
  http_parser: '2.8.0',
  icu: '61.1',
  modules: '64',
  napi: '3',
  nghttp2: '1.32.0',
  node: '10.4.1',
  openssl: '1.1.0h',
  tz: '2018c',
  unicode: '10.0',
  uv: '1.20.3',
  v8: '6.7.288.45-node.7',
  zlib: '1.2.11' }

$ node -p process.platform
win32

Side issue

postpack has only a single sentence of documentation which gives no indication that it would run in any circumstances other than after npm pack. In contrast, the npm blog states that “postpack … will run on both npm pack and npm publish”. I find this behavior surprising and if it is true, the documentation needs to be clarified.


(Kat Marchán) #2

Ahhh. This is definitely a bug. I’m not sure what got messed up. Pretty sure this is a side effect of the last time I juggled that stuff around to add publish previews.

It would be awesome to get a PR to fix this and I don’t think it would be a lot of work. You can look in lib/pack.js and look at the packDirectory function and its callsites. I can answer any questions. Marking this one as a good first PR because I think it’s a good mix of code reading and can be fixed with moving only a little bit of code around.


(Lars Willighagen) #3

Writing for npm pack is currently done right outside packDirectory, where --dry-run is handled. This means (AFAICT) either moving --dry-run support to inside packDirectory or moving the postpack hook to outside packDirectory. Since --dry-run handling is only done on one of the three places packDirectory is called moving that would be a bit hack-y, or require some more insight into future plans for --dry-run than I currently have. However, moving the postpack hook to outside packDirectory means having to maintain basically the same code on three places, and asymmetry with prepack unless that’s moved outside too (more maintaining :neutral_face:). On the other hand, maybe I’m overthinking this. Would an optional --dry-run argument to packDirectory be enough, or is that too much of a quick fix?


(Kat Marchán) #4

i dunno, let’s see what it looks like. I’m not opposed to optional flags, specially for mostly internal functions. :slight_smile:


(Lars Willighagen) #5

(Kat Marchán) #6

https://github.com/npm/cli/pull/47 has been merged and will be included in the next npm release. Thank you, @larsgw!


(system) #7

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.