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

Publish old versions without touching latest tag #472

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jablko
Copy link
Contributor

@jablko jablko commented May 30, 2022

Use the defaultTag option when publishing old versions to avoid changing the latest tag and then having to undo and retag the latest version:

// If this is an older version of the package, we still update tags for the *latest*.
// NPM will update "latest" even if we are publishing an older version of a package (https://github.com/npm/npm/issues/6778),
// so we must undo that by re-tagging latest.
await addNpmTagsForPackage(latestVersion, latestVersionString, client, log, dry);

I switched to using libnpmpublish because I couldn't pass the defaultTag option to npm-registry-client's publish() method. libnpmpublish is what the npm CLI's publish command now uses.

@jablko jablko force-pushed the patch-54 branch 2 times, most recently from d36ff5b to 8ce850e Compare June 5, 2022 15:09
@jablko jablko changed the title Publish older versions without touching latest tag Publish old versions without touching latest tag Jun 23, 2022
@jablko jablko force-pushed the patch-54 branch 2 times, most recently from 1d54b72 to 383193f Compare June 29, 2022 18:00
"tar-stream": "^3.1.6",
"which": "^4.0.0"
},
"devDependencies": {
"@types/charm": "^1.0.6",
"@types/tar": "^6.1.9",
"@types/tar-stream": "^3.1.3",
"@types/which": "^3.0.2",
"@qiwi/npm-types": "^1.0.3"
Copy link
Member

Choose a reason for hiding this comment

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

wait, wasn't this PR supposed to get rid of the @qiwi dep?

Copy link
Member

Choose a reason for hiding this comment

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

This is one step, I have more changes I was building on this PR; probably I'll send a new one instead that includes this one.

const tarData = await pacote.tarball(packageDir);
// Make sure we never assign the latest tag to an old version.
if (!dry)
await libpub.publish(manifest ? { ...manifest, bundledDependencies: undefined } : manifest, tarData, {
Copy link
Member

Choose a reason for hiding this comment

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

why might manifest be undefined? Should we publish if it's missing?

I noticed this because I'd prefer to pull out a manifest.bundledDependencies = undefined instead of a spread, but that doesn't work if it's possibly undefined.

@@ -10,13 +10,16 @@
"@definitelytyped/header-parser": "workspace:*",
"@definitelytyped/retag": "workspace:*",
"@definitelytyped/utils": "workspace:*",
"libnpmpublish": "^9.0.3",
Copy link
Member

Choose a reason for hiding this comment

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

I thought this change would let us stop depending on npm-registry-publish, but it just adds the new dependency.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's half a change; this PR removed some of the registry dep but not all.


export const cacheDir = joinPaths(process.env.GITHUB_ACTIONS ? joinPaths(__dirname, "../../..") : os.tmpdir(), "cache");

type NeedToFixNpmRegistryClientTypings = any;

export class NpmPublishClient {
Copy link
Member

Choose a reason for hiding this comment

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

this only tags and deprecates now. I'd rename to NpmTagger or something
and we seem to mostly create it on-demand, so maybe callers should switch to calling tag/deprecate directly.

Comment on lines +49 to +50
const token = await getSecret(Secret.NPM_TYPES_TOKEN);
const client = new NpmPublishClient(token);
Copy link
Member

Choose a reason for hiding this comment

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

splitting this statement seems unnecessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants