Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,52 @@ describe(`publish`, () => {
});
}));

test(`should support --dry-run flag`, makeTemporaryEnv({
name: `dry-run-test`,
version: `1.0.0`,
}, async ({path, run, source}) => {
await run(`install`);

const {stdout} = await run(`npm`, `publish`, `--dry-run`, `--tolerate-republish`);
expect(stdout).toContain(`[DRY RUN]`);
}));

test(`should support --json flag`, makeTemporaryEnv({
name: `json-test`,
version: `1.0.0`,
}, async ({path, run, source}) => {
await run(`install`);

const {stdout} = await run(`npm`, `publish`, `--json`, `--dry-run`, `--tolerate-republish`);
const result = JSON.parse(stdout);
expect(result).toHaveProperty(`name`, `json-test`);
expect(result).toHaveProperty(`version`, `1.0.0`);
expect(result).toHaveProperty(`dryRun`, true);
}));

test(`should support --registry flag`, makeTemporaryEnv({
name: `registry-test`,
version: `1.0.0`,
}, async ({path, run, source}) => {
await run(`install`);

const {stdout} = await run(`npm`, `publish`, `--json`, `--dry-run`, `--registry`, `https://registry.npmjs.org`, `--tolerate-republish`);
const result = JSON.parse(stdout);
expect(result).toHaveProperty(`registry`, `https://registry.npmjs.org`);
}));

test(`should support directory argument`, makeTemporaryEnv({
name: `directory-test`,
version: `1.0.0`,
}, async ({path, run, source}) => {
await run(`install`);

const {code} = await run(`npm`, `publish`, path, `--dry-run`, `--tolerate-republish`, {
cwd: npath.dirname(path),
});
expect(code).toBe(0);
}));

testIf(
() => !!process.env.ACTIONS_ID_TOKEN_REQUEST_URL,
`should publish a package with a valid provenance statement`,
Expand Down
248 changes: 181 additions & 67 deletions packages/plugin-npm-cli/sources/commands/npm/publish.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {BaseCommand, WorkspaceRequiredError} from '@yarnpkg/cli';
import {Configuration, MessageName, Project, ReportError, StreamReport, scriptUtils, miscUtils} from '@yarnpkg/core';
import {ppath} from '@yarnpkg/fslib';
import {npmConfigUtils, npmHttpUtils, npmPublishUtils} from '@yarnpkg/plugin-npm';
import {packUtils} from '@yarnpkg/plugin-pack';
import {Command, Option, Usage, UsageError} from 'clipanion';
Expand Down Expand Up @@ -46,12 +47,30 @@ export default class NpmPublishCommand extends BaseCommand {
description: `Generate provenance for the package. Only available in GitHub Actions and GitLab CI. Can be set globally through the \`npmPublishProvenance\` setting or the \`YARN_NPM_CONFIG_PROVENANCE\` environment variable, or per-package through the \`publishConfig.provenance\` field in package.json.`,
});

dryRun = Option.Boolean(`--dry-run`, false, {
description: `Show what would be published without actually publishing`,
});

json = Option.Boolean(`--json`, false, {
description: `Output the result in JSON format`,
});

registry = Option.String(`--registry`, {
description: `The registry to publish to`,
});

directory = Option.String({
description: `The directory to publish (defaults to current directory)`,
required: false,
});

async execute() {
const configuration = await Configuration.find(this.context.cwd, this.context.plugins);
const {project, workspace} = await Project.find(configuration, this.context.cwd);
const cwd = this.directory ? ppath.resolve(this.context.cwd, this.directory) : this.context.cwd;
const configuration = await Configuration.find(cwd, this.context.plugins);
const {project, workspace} = await Project.find(configuration, cwd);

if (!workspace)
throw new WorkspaceRequiredError(project.cwd, this.context.cwd);
throw new WorkspaceRequiredError(project.cwd, cwd);

if (workspace.manifest.private)
throw new UsageError(`Private workspaces cannot be published`);
Expand All @@ -64,85 +83,180 @@ export default class NpmPublishCommand extends BaseCommand {
const ident = workspace.manifest.name;
const version = workspace.manifest.version;

const registry = npmConfigUtils.getPublishRegistry(workspace.manifest, {configuration});
const registry = this.registry || npmConfigUtils.getPublishRegistry(workspace.manifest, {configuration});

// For JSON output, we collect data differently but use the same core logic
if (this.json) {
const result = await this.executeWithCollectedOutput(workspace, registry, configuration, ident, version);
this.context.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
return 0;
}

const report = await StreamReport.start({
configuration,
stdout: this.context.stdout,
}, async report => {
// Not an error if --tolerate-republish is set
if (this.tolerateRepublish) {
try {
const registryData = await npmHttpUtils.get(npmHttpUtils.getIdentUrl(ident), {
configuration,
registry,
ident,
jsonResponse: true,
});

if (!Object.hasOwn(registryData, `versions`))
throw new ReportError(MessageName.REMOTE_INVALID, `Registry returned invalid data for - missing "versions" field`);

if (Object.hasOwn(registryData.versions, version)) {
report.reportWarning(MessageName.UNNAMED, `Registry already knows about version ${version}; skipping.`);
return;
}
} catch (err) {
if (err.originalError?.response?.statusCode !== 404) {
throw err;
}
}
}
await this.executeCore(workspace, registry, configuration, ident, version, report);
});

return report.exitCode();
}

await scriptUtils.maybeExecuteWorkspaceLifecycleScript(workspace, `prepublish`, {report});
private async executeCore(workspace: any, registry: string, configuration: any, ident: any, version: string, report: any) {
Copy link
Owner

Choose a reason for hiding this comment

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

@copilot now that we have one execute path, we don't need executeCore anymore

Copy link
Author

Choose a reason for hiding this comment

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

Removed the unnecessary executeCore method and consolidated all logic into a single execution path. The duplicate method has been eliminated while maintaining identical functionality. See commit 22b0854.

Copy link
Owner

Choose a reason for hiding this comment

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

@copilot we also factored out methods like checkTolerateRepublish, determineProminence, reportProminenceDesision, and performPackAndPublish. That resulted in any types. Now that we have one execute flow again, can we just fold those back into the one method?

Copy link
Author

Choose a reason for hiding this comment

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

Folded all extracted methods back into the single execute method to eliminate the any types. The code now has proper TypeScript typing since all variables are declared in the correct scope. See commit ee34047.

// Check if we should skip republishing
const shouldSkip = await this.checkTolerateRepublish(ident, version, configuration, registry);
if (shouldSkip) {
report.reportWarning(MessageName.UNNAMED, `Registry already knows about version ${version}; skipping.`);
return;
}

await packUtils.prepareForPack(workspace, {report}, async () => {
const files = await packUtils.genPackList(workspace);
await scriptUtils.maybeExecuteWorkspaceLifecycleScript(workspace, `prepublish`, {report});

for (const file of files)
report.reportInfo(null, file);
await packUtils.prepareForPack(workspace, {report}, async () => {
await this.performPackAndPublish(workspace, registry, configuration, ident, report);
});

const pack = await packUtils.genPackStream(workspace, files);
const buffer = await miscUtils.bufferStream(pack);
const message = this.dryRun
? `[DRY RUN] Package publication completed`
: `Package archive published`;
report.reportInfo(MessageName.UNNAMED, message);
}

const gitHead = await npmPublishUtils.getGitHead(workspace.cwd);
private async executeWithCollectedOutput(workspace: any, registry: string, configuration: any, ident: any, version: string) {
const result: any = {
name: ident.name,
version,
registry,
dryRun: this.dryRun,
};

let provenance = false;
if (workspace.manifest.publishConfig && `provenance` in workspace.manifest.publishConfig) {
provenance = Boolean(workspace.manifest.publishConfig.provenance);
if (provenance) {
report.reportInfo(null, `Generating provenance statement because \`publishConfig.provenance\` field is set.`);
} else {
report.reportInfo(null, `Skipping provenance statement because \`publishConfig.provenance\` field is set to false.`);
try {
// Check if we should skip republishing
const shouldSkip = await this.checkTolerateRepublish(ident, version, configuration, registry);
if (shouldSkip) {
result.warning = `Registry already knows about version ${version}; skipping.`;
return result;
}

// Create a mock report that collects data instead of logging
const dataCollector = {
files: [] as Array<string>,
reportInfo: (name: any, file: string) => {
if (file) {
result.files = result.files || [];
result.files.push(file);
}
} else if (this.provenance) {
provenance = true;
report.reportInfo(null, `Generating provenance statement because \`--provenance\` flag is set.`);
} else if (configuration.get(`npmPublishProvenance`)) {
provenance = true;
report.reportInfo(null, `Generating provenance statement because \`npmPublishProvenance\` setting is set.`);
}

const body = await npmPublishUtils.makePublishBody(workspace, buffer, {
access: this.access,
tag: this.tag,
registry,
gitHead,
provenance,
});

await npmHttpUtils.put(npmHttpUtils.getIdentUrl(ident), body, {
configuration,
registry,
ident,
otp: this.otp,
jsonResponse: true,
});
},
reportWarning: () => {},
reportError: () => {},
};

await packUtils.prepareForPack(workspace, dataCollector, async () => {
const publishData = await this.performPackAndPublish(workspace, registry, configuration, ident, dataCollector);
Object.assign(result, publishData);
});

report.reportInfo(MessageName.UNNAMED, `Package archive published`);
result.message = this.dryRun
? `Package publication completed (dry run)`
: `Package archive published`;

return result;
} catch (error) {
result.error = error.message;
return result;
}
}

private async checkTolerateRepublish(ident: any, version: string, configuration: any, registry: string): Promise<boolean> {
if (!this.tolerateRepublish) return false;

try {
const registryData = await npmHttpUtils.get(npmHttpUtils.getIdentUrl(ident), {
configuration,
registry,
ident,
jsonResponse: true,
});

if (!Object.hasOwn(registryData, `versions`))
throw new ReportError(MessageName.REMOTE_INVALID, `Registry returned invalid data for - missing "versions" field`);

return Object.hasOwn(registryData.versions, version);
} catch (err) {
if (err.originalError?.response?.statusCode !== 404)
throw err;
return false;
}
}

private determineProvenance(workspace: any, configuration: any): boolean {
if (workspace.manifest.publishConfig && `provenance` in workspace.manifest.publishConfig)
return Boolean(workspace.manifest.publishConfig.provenance);
if (this.provenance)
return true;
if (configuration.get(`npmPublishProvenance`))
return true;
return false;
}

private reportProvenanceDecision(provenance: boolean, workspace: any, report: any) {
if (workspace.manifest.publishConfig && `provenance` in workspace.manifest.publishConfig) {
const message = provenance
? `Generating provenance statement because \`publishConfig.provenance\` field is set.`
: `Skipping provenance statement because \`publishConfig.provenance\` field is set to false.`;
report.reportInfo(null, message);
} else if (this.provenance) {
report.reportInfo(null, `Generating provenance statement because \`--provenance\` flag is set.`);
} else if (provenance) {
report.reportInfo(null, `Generating provenance statement because \`npmPublishProvenance\` setting is set.`);
}
}

private async performPackAndPublish(workspace: any, registry: string, configuration: any, ident: any, report: any) {
const files = await packUtils.genPackList(workspace);

// Report files if this is a streaming report
if (report.reportInfo && typeof report.reportInfo === `function`) {
for (const file of files) {
report.reportInfo(null, file);
}
}

const pack = await packUtils.genPackStream(workspace, files);
const buffer = await miscUtils.bufferStream(pack);
const gitHead = await npmPublishUtils.getGitHead(workspace.cwd);
const provenance = this.determineProvenance(workspace, configuration);

// Report provenance decision if this is a streaming report
if (report.reportInfo && typeof report.reportInfo === `function`)
this.reportProvenanceDecision(provenance, workspace, report);

const body = await npmPublishUtils.makePublishBody(workspace, buffer, {
access: this.access,
tag: this.tag,
registry,
gitHead,
provenance,
});

return report.exitCode();
if (!this.dryRun) {
await npmHttpUtils.put(npmHttpUtils.getIdentUrl(ident), body, {
configuration,
registry,
ident,
otp: this.otp,
jsonResponse: true,
});
} else if (report.reportInfo && typeof report.reportInfo === `function`) {
report.reportInfo(MessageName.UNNAMED, `[DRY RUN] Package would be published to ${registry}`);
}

// Return data for JSON output
return {
files,
gitHead,
provenance,
published: !this.dryRun,
};
}
}