feat(pack): add package filtering for pack command (#4351)#9477
feat(pack): add package filtering for pack command (#4351)#9477
Conversation
| if (opts.recursive) { | ||
| const selectedProjectsGraph = opts.selectedProjectsGraph as ProjectsGraph | ||
| const pkgs = Object.values(selectedProjectsGraph).map((wsPkg) => wsPkg.package) | ||
| const pkgsToPack = await pFilter(pkgs, async (pkg) => { |
There was a problem hiding this comment.
Why do you need pFilter and async here? pkgs is not an array of Promises.
| const chunks = sortPackages(selectedProjectsGraph) | ||
|
|
||
| for (const chunk of chunks) { | ||
| for (const pkgDir of chunk) { |
There was a problem hiding this comment.
Just copied from recursivePublish. Parallel running has been added for pack and publish. I found that there was an issue before, which may need to be reviewed: #6968.
| ${tarballPath}` | ||
| const printPackResult = function (packResultJson: PackResultJson): string { | ||
| const { name, version, filename, files } = packResultJson | ||
| return `${opts.unicode ? '📦 ' : 'package:'} ${name}@${version} |
There was a problem hiding this comment.
We don't use emojis in any output currently. I don't know if we should start using them.
There was a problem hiding this comment.
I added the emojis for consistency with npm, and for human readability, it does make it easier to find the package in the console log. No other ideas.
See https://github.com/npm/cli/blob/latest/lib/utils/tar.js#L16
0f303ad to
2208904
Compare
| out: opts.out ? path.join(opts.dir, opts.out) : undefined, | ||
| packDestination: !opts.out ? path.join(opts.dir, opts.packDestination ?? '.') : undefined, |
There was a problem hiding this comment.
The packages will be written relative to the current working directory? This is not where the packages are saved currently with non-recursive pack. It is saved relative to the package's directory
| out: opts.out ? path.join(opts.dir, opts.out) : undefined, | |
| packDestination: !opts.out ? path.join(opts.dir, opts.packDestination ?? '.') : undefined, | |
| out: opts.out ? path.join(pkg.rootDir, opts.out) : undefined, | |
| packDestination: !opts.out ? path.join(pkg.rootDir, opts.packDestination ?? '.') : undefined, |
There was a problem hiding this comment.
Both are fine. I personally prefer relative to the current working directory, so that all tarballs are in one directory, which is the same behavior as npm workspace. But there are also some package managers such as maven, which put their build products in a specific target directory of the package by default.
Consider the following command
pnpm pack -r --pack-destination=out
or
npm config set pack-destination="${CI_PROJECT_DIR}/out/" (it doesn't work, I'm trying to fix it, and path.join needs to be changed to path.resolve to support absolute paths)
I hope the final result is:
out/a.tgz
out/b.tgz
If it is relative to the package directory, the result will be
packages/a/out/a.tgz
packages/b/out/b.tgz
This does not match the pack-destination configuration
There was a problem hiding this comment.
Making it relative to workspaceDir ?? lockfileDir might be OK. But making it relative to current working directory is not deterministic. Also, as you mentioned, it should work fine with absolute path.
There was a problem hiding this comment.
No. If we need to support relative paths, we should make it relative to the cwd, not the workspaceDir.
in <root>/packages/a run pnpm pack -F a --pack-destination=./out
relative to cwd: <root>/packages/a/out/a.tgz
relative to workspaceDir: <root>/out/a.tgz (this is not in line with common practice)
in <root>/packages/a run pnpm pack -F a -F b --pack-destination=./out
relative to cwd:
<root>/packages/a/out/a.tgz
<root>/packages/a/out/b.tgz (this might be a bit weird, but it's ok)
relative to workspaceDir:
<root>/out/a.tgz (this is not in line with common practice)
<root>/out/b.tgz
in <root> run pnpm pack -F a --pack-destination=./out
<root>/out/a.tgz
There was a problem hiding this comment.
I create a new pull request for supporting get pack-destination from npmrc
There was a problem hiding this comment.
We are not adding new settings to that package. New settings are added to the config package in this repo.
There was a problem hiding this comment.
Added pack-destination to the config package 2dc01b3
2871c60 to
2dc01b3
Compare
|
merged |
…9477) * feat(pack): add package filtering for pack command (pnpm#4351) * feat(plugin-commands-publishing): parallelly run recusive pack and publish * refactor: pack.ts * feat(pack): support absolute pack-destination path * feat: get `pack-destination` configuration from npmrc * refactor: pack.ts * docs: update changeset * refactor: pack.ts close pnpm#4351 --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
Add
--filterand--recursiveforpackcommandReleated #4351 #4792 #3290
I have been waiting for this feature for a long time. The current version of pnpm does not support package filtering for the
packcommand, I mean--filterand--recursive. We need this feature to help reviewing the contents of the packages before they are published to the registry.This PR changes:
1 Console report
For single-package mode, the
--jsonreport will remain as is to maintain compatibility. For multi-package mode, the--jsonreport will return an array (this may be a breaking change). You can use this array to check the packages info.The readable report style is changed to be consistent with the
npm pack --jsoncommand, a title is added to each module for quick inspection.2 Output path
--outputtakes precedence over--pack-destinationbecause--outputis more precise.If neither is specified, the current package's directory is used, and is passed to other packages so that all tarballs are generated in the same path.
3 Difference from
publish --recursiveSince
packcommand only performs packaging and does not actually publish tarballs to the registry, it does not check the registry and does not care whether the package manifest has theprivatefield.