Bug: output published is always false when used with changeset publish --no-git-tag

[akphi]

@Andarist While testing out my change in #130 I found out that when I set my publish script to use changeset publish --no-git-tag, the output published is always false. This is due to the following piece of code:

action/src/run.ts

Lines 145 to 155 in e8ffc2d

	if (releasedPackages.length) {
	return {
	published: true,
	publishedPackages: releasedPackages.map((pkg) => ({
	name: pkg.packageJson.name,
	version: pkg.packageJson.version,
	})),
	};
	}
	return { published: false };

releasedPackages is only populated when the text New tag: is found in the CLI output. I'm not sure if this is still the right indicator for successful publishing, instead, I think we should fix it so that as long as no error thrown when running the publishing script, we should set the output published to true.

I believe a lot of people depends on this output flag in their github actions script, especially to distinguish between publish flow vs. creating the new version PR flow. I have suggested what I think is a potential fix in #142, let me know what you think. Thanks!

[akphi]

@Andarist Is it ok to add a flag pushGitTags (similar to how we did createGithubReleases flag in #130 ) as a workaround for people who want to use --no-git-tag? - PR #141

[Andarist]

I believe a lot of people depends on this output flag in their github actions script, especially to distinguish between publish flow vs. creating the new version PR flow.

This is covered by hasChangesets output:
https://github.com/changesets/action/pull/136/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R165

Does this satisfy your use case or do you need to know more about what has been done by changeset publish?

releasedPackages is only populated when the text New tag: is found in the CLI output.

I agree that it's not the best way to handle this - I'm unsure what's the best way to share a better "structured" output from a CLI tool? I guess something like --json would help here?

I think we should fix it so that as long as no error thrown when running the publishing script, we should set the output published to true.

That wouldn't quite work as we call changeset publish without knowing if something needs to be published or not - the script can just bail out cause everything is already published and in such a situation ending up with published: true would be rather confusing.

[akphi]

Does this satisfy your use case or do you need to know more about what has been done by changeset publish?

Essentially, that's my problem, I need to know when changeset publish did something, i.e. something was published to NPM for example, but I don't want to push any tags to Github, that's why I tried to propose #143

I agree that it's not the best way to handle this - I'm unsure what's the best way to share a better "structured" output from a CLI tool? I guess something like --json would help here?

I barbarically proposed #142 but then I realized the flow is a lot more complicated than that, changesets publish only deal with generic CLI and has not assumption about its running in a Github CI, so to hook it with this action is not straight-forward. Maybe something like changesets publish will output some file, and then this action will try to parse that file.

[Andarist]

changesets publish only deal with generic CLI and has not assumption about its running in a Github CI, so to hook it with this action is not straight-forward. Maybe something like changesets publish will output some file, and then this action will try to parse that file.

I'm not 100% sure if I understand this so if you could rephrase I would appreciate it. I might just reiterate what you have said here but as I'm not sure if we are on the same page I'll try to add some context.

If you provide a custom publish script then we expect it to forward what Changesets writes to stdout so we could process it. If you don't invoke Changesets in your script then we just expect that tool to implement the same shape of the output (which is under-specified).

However we'd decide to pass the information around this would always require the user to conform to our expected shape, it's like a communication protocol between the script and this action.

Essentially, that's my problem, I need to know when changeset publish did something, i.e. something was published to NPM for example, but I don't want to push any tags to Github

To understand your use case better - why don't you want to create git tags?

[akphi]

changesets publish only deal with generic CLI and has not assumption about its running in a Github CI, so to hook it with this action is not straight-forward. Maybe something like changesets publish will output some file, and then this action will try to parse that file.

So since on changesets/changeset side, we don't want any Github specific logic, I am thinking about creating a flag to output the publishing result to a file. Then changesets/action can pick up this file and parse the content. I suppose this gives us more control and accuracy than scraping the CLI std output.

To understand your use case better - why don't you want to create git tags?

We have a fair amount of packages (~30) and we release quite frequently (partly because we use updateInternalDependents=always), so we start noticing our repo quickly building up a lot of tags over time. Now in the past, where Github Release is not that well-organized, it makes it hard for us to find changelogs that we really care about. I think with the recent change on Github Release, things have been improved. Also we have 2 types of packages: libraries and web application, we want the versions of application to drive the release, so we want to say we're releasing version 2.0.0 or 3.0.0, those are the only tags we would like to create.

Like said, I do understand our workflow might not be common enough to drive changes like #143 but I am doing this to make use of --no-git-tag from changeset publish. If you deem that irrelevant/not-reasonable, I totally respect your position.

Let me know what you think! Thanks!

[Andarist]

I think that I would still prefer to output stuff to the CLI stdout - while knowing that it can sometimes be a little bit problematic.

In the past, I've created such an issue about introducing --json flag. I think that if this would be implemented then we could use that flag here. This would skipping tag creation with --no-git-tag while retaining the info about the released packages.

[akphi]

Gotcha, closed #143

[Andarist]

q: are u interested in implementing that —json flag?

[akphi]

sure thing, why not 😃 curious though, how do you envision us scanning those JSON from the stdout?

[Andarist]

Just grab the stdout and parse it - we can also be a little bit more resistant and use a startegy like this one: https://github.com/changesets/changesets/pull/676/files

[akphi]

I bumped into a small problem. So we're capturing the output of the publish script, for me, my script is something like

yarn publish:prepare && changeset publish

so the output looks like

➤ YN0000: [@finos/babel-preset-legend-studio]: Process started
➤ YN0000: [@finos/babel-preset-legend-studio]: Process exited (exit code 0), completed in 1s 690ms
➤ YN0000: [@finos/eslint-plugin-legend-studio]: Process started
➤ YN0000: [@finos/eslint-plugin-legend-studio]: Process exited (exit code 0), completed in 1s 681ms
➤ YN0000: [@finos/legend-dev-utils]: Process started
➤ YN0000: [@finos/legend-dev-utils]: Process exited (exit code 0), completed in 1s 637ms
...
🦋  info npm info @finos/babel-preset-legend-studio
🦋  info npm info @finos/eslint-plugin-legend-studio
🦋  info npm info @finos/legend-application
🦋  info npm info @finos/legend-art
🦋  info npm info @finos/legend-dev-utils
...
success packages published successfully:
🦋  @finos/babel-preset-legend-studio@0.0.34
🦋  @finos/eslint-plugin-legend-studio@0.1.9
🦋  @finos/legend-application@1.1.2
🦋  @finos/legend-art@0.3.2
🦋  @finos/legend-dev-utils@0.3.7
...

Point being, there are a lot of non-JSON output before the relevant output of changeset publish --json, so I'm thinking maybe instead of my preventing the logging of those output, I will output the JSON last, separated from the output above it by something special, maybe

... (previous output)

--- JSON OUTPUT ---
{...}

What do you think? In a way, this is not like the behavior of eslint --format=json or yarn config --json or so because the stdout does not consist solely of JSON.

---

UPDATE: I see getLastJsonObjectFromString from changesets/changesets#676. Maybe we can, but we can't be sure the output of the publish script can contain { or not...

[Andarist]

UPDATE: I see getLastJsonObjectFromString from changesets/changesets#676. Maybe we can, but we can't be sure the output of the publish script can contain { or not...

--json handling would be "optional". Even when we call the regular changeset publish --json we can't be sure if w receive JSON in return because we might call an earlier version of Changesets (we should recheck if @changesets/cli just ignores unknown flags instead of throwing on them - IIRC though they are simply ignored).

If we can't retrieve the JSON or if if the retrieved JSON is incorrect then we should fallback to the current extraction logic based on New tag: etc

[luizstacio]

We faced this issue recently, and we ended up changing the way the published and outputs are generated; the thing we thought was that the New tag: is not the best way to match published but instead just get the message published success and the packages names.
FuelLabs@adb410c

[Andarist]

the thing we thought was that the New tag: is not the best way to match published

I agree, although in general, I would like to figure out a better way to share the structured output between the CLI and the and changesets/action. Parsing the stdout will always be at least a little brittle

[luizstacio]

the thing we thought was that the New tag: is not the best way to match published

I agree, although in general, I would like to figure out a better way to share the structured output between the CLI and the and changesets/action. Parsing the stdout will always be at least a little brittle

One idea would be use process messages. It can be a better way to integrate.

[sndrs]

We have several teams that use changeset version --snapshot to create canary/test releases from PRs via a GitHub action that's triggered by the presence of a label (e.g. here).

Once they're out, we add a comment to the PR with a list of packages and versions that have been published. To enable this, we can't use --no-git-tag, because we need to get the releases in a subsequent step.

These are ephemeral releases that we don't need to keep around, but right now they go into git history alongside production releases.

It would be great if we could keep these releases out of git but still get the list of releases from changesets/action.