[publish] Keep lock files in cleanup.

[avocadowastaken]

np not removes lock files and removes node_modules only when there are no any of these files:

https://github.com/sindresorhus/np/blob/64a73f618c07042436d50f8747821c2b6b0d2cca/source/index.js#L110-L137

But pika-pack publish removes everything, which can lead to different kind of troubles.

https://github.com/pikapkg/pack/blob/ef6d8fd0295983ab9249021d35de0d0cfa9867c3/src/commands/publish.ts#L113-L125

I would happy to add np adaptation in PR.

[FredKSchott]

This behavior came from np originally, I wonder if they changed it. I

actually agree with the reasoning behind deleting the lockfile: your package consumers will be getting the most up to date dependencies with your package, so before publishing you better run tests against the same dependencies that your consumers will get. Deleting the lockfile and re-installing deps imitates this.

Does anyone have a link to the discussion in np where this behavior was changed?

[avocadowastaken]

your package consumers will be getting the most up to date dependencies with your package, so before publishing you better run tests against the same dependencies that your consumers will get

Oh, that's reasonable. But still, after publish I have changed yarn.lock file, which is confusing.

Maybe we can change to:

if (!hasLockFile) {
  await fs.unlink("node_modules");
}

if (options.yarn) {
  await execa("yarn", ["upgrade"]);
} else {
  await execa(
    "npm",
    hasLockFile
      ? ["audit", "fix"]
      : ["install", "--no-production", "--no-package-lock"]
  );
}

if (hasLockFile) {
  const { stdout: status } = await execa("git", ["status", "--porcelain"]);
  if (status !== "") {
    throw new Error(
      options.yarn
        ? "yarn.lock was updated during cleanup, run yarn upgrade before publishing."
        : "package-lock.json was updated during cleanup, run npm audit fix before publishing"
    );
  }
}

[timhudson]

Does anyone have a link to the discussion in np where this behavior was changed?

sindresorhus/np#285

[timhudson]

Testing dependencies resolved from package.json feels like a must before publishing. Reproducible builds are very useful in development and CI but can mask reality. I've typically been running npm ci when testing feature branches and npm install when testing before a deploy or publishing.

One nice thing about yarn install --frozen-lockfile and npm ci is they will both fail when the lock-file is unable to satisfy package.json needs. This can be a useful way to ensure that reproducible builds don't drift too far from fresh installs. But this is more of a workflow and maintenance issue. Should it block publishing?

I don't think it should, but I do think there is value in guiding or even automating the maintenance of lock files. I'm tempted to play around with automatically updating and commiting lock-file changes in the publish flow. It seems like the right time to do it. Once we've confirmed that tests pass with the latest dependency tree and we're ready to publish to users, automatically updating the lock-file to more closely match users feels like a reasonable thing to do.

Manual maintenance of lock files is often a source of frustration and confusion. Perhaps we can help alleviate some of that.

[avocadowastaken]

The simplest solution I see is to remove --force option from npm version and run it after tests step. So it will reassure that working tree is clean after cleanup and tests.