thefrontside / actions Goto Github PK

I wanted to double check that the install instructions that are generated on our pull requests are accurate. Installing using version number (as shown in the screenshot above) works fine. However, now that we've incorporated tags to the publishing process, I was trying to install packages using tags and came across an issue.

For the most part there are no problems using npm install <package>@<tag>, but installing packages with tags that have slashes results in this error:

What should we do about this? Is there a workaround for this issue?

In this pull request workflow, the comment failed to post to GitHub. It looks like the comment token is misconfigured, but the workflow still succeeded

https://github.com/microstates/microstates.js/commit/fe17889f91ba317930a070a9f7f0b7399a676917/checks?check_suite_id=260590046#step:8:23

Context

synchronize-with-npm works by comparing the latest commit to the one previous to that one to determine which packages to publish.

However, we noticed when a pull request isn't squashed before getting merged, the merge commit will compare itself with the previous commit which in the synchronize-with-npm action would result in no changes and therefore not publish any packages.

To mitigate this oversight, we have come up with a proposal:

Current Action Workflow

• get all the files that have changed
• find all the relevant package.json of those files to determine from where the action must run publish
• filter out the directories from the default list and ignore arguments from the workflow
• loop over the list of directories
  • skip if private or if the package version has already been published
  • publish and push git tag

Possible Solution

• find all package.json in the repository (excluding those in node_modules)
• remove private packages
• check each package@version on npm registry to determine if it should publish

Why Did It Filter In The First Place?

We have an action for publishing previews which would benefit from knowing which files have changed so that we don't publish every package for each pull request for each commit. Then the synchronize-with-npm action was born and took a lot of its logic from the preview action, but now we realize the filtering is not necessary for publishing releases.

Any Downfalls?

We can still integrate the ignore argument to prevent certain directories and its sub-directories from publishing but that seems redundant because users will be expected to be more explicit with which packages they want publishing so it'll be rare for a package to be published by mistake. The same goes for the logic of preventing sub-directory packages from publishing; this is actually a plus because the new approach would resolve this issue and get rid of that caterpillar.

The release action run here https://github.com/microstates/microstates.js/commit/5ba88cd2614c340bb1f62fd60184505a47608739/checks?check_suite_id=257270796 failed because of what appears to be the -a option being passed to git tag which creates an "annotated" tag and requires input on what the annotation is. Without the -m flag it will open up an editor, but since this is a CI env it fails.

I think this is unnecessary and can be removed.

https://github.com/thefrontside/actions/blob/master/synchronize-with-npm/entrypoint.sh#L28-L29

Also, the git command to push the tags does not need to push any branches, so it should be

$ git push "https://$GITHUB_ACTOR:[email protected]/$GITHUB_REPOSITORY.git" --tags

TL;DR: ${NPM_TOKEN} must be passed into every workflow run if there is authentication configuration in the .npmrc file inside a repository regardless of if they need to authenticate.

Context

Past

Instead of expecting the user to configure and keep an .npmrc file on their repository, we allowed our action to be configured with arguments so that the action can automate the scoping/authenticating process by creating/appending the file in the workflow and have it automatically discarded by Github Actions. This is still the case for v1.1.

Current

From v1.2 we took away all those arguments and now expect the user to setup any scoping and authenticating on their own by creating an .npmrc file and passing in the corresponding secrets from the workflow to the action. By default we use NPM_TOKEN environment variable to authenticate for npmjs so for most use cases the user would not be required to keep an .npmrc file on their repository.

However, I've come across an for resideo/blueprint and resideo/zeus (the two projects that are using our actions and also that requires scoping/authentication configuration set in their repository).

Problem

There is an error when there is authenticating configuration in the .npmrc file of a repository and a workflow does not expose its corresponding variable. For example, if the .npmrc file has //registry.npmjs.org:_authToken=${TOKEN_NAME}, a workflow must expose that token in its actions or else we get a Could not replace ${TOKEN_NAME} error.

This is okay for the most part. We should be exposing that variable anyway if we're trying to publish or do anything else that requires authentication. The problem is when there is a workflow that does not need to be authenticated. We still receives that error because we did not pass in the token as an environment variable.

We've just never come across this issue before because resideo projects have their authentication set with GITHUB_TOKEN which was always being exposed.

Possible Solutions

Bad-Solutions 👎👎👎

• Provide ${NPM_TOKEN} to every workflow.
• Not sure if possible but the .npmrc could have a conditional where it detects for NPM_TOKEN and if it comes back null, we can set it to undefined to prevent the error.
• The same conditional could be applied from the action but this would need to be put in every action we make.

Hmmm?-Solution 🤷‍♂🤷‍♂🤷‍♂

• Introduce authentication argument. Unlike last time, we won't try to auto detect from package.json. We could just do authentication: //registry.npmjs.org:_authToken=${{ secrets.NPM_TOKEN }} as an argument and the actions that require authenticating can do echo ${INPUT_AUTHENTICATION} >> ~/.npmrc. This will mean user will still need scoping configurations in the .npmrc file on their repo. And the authenticating part can still be figure-it-out-yourself and not be spoon-fed to the user.
  • Or just do REGISTRY so we do echo ${INPUT_REGISTRY}${{ secrets.NPM_TOKEN }} > ~/.npmrc.

It looks like we're detecting incorrect SHA. What's up with that?

The purpose of this issue is to take inventory of all of our github actions so we can incorporate them into covector. Under each section I've written a brief summary and for some of the more complex actions I jot down all of its steps.

Yarn

This action was created to replace running yarn install directly inside a Github Actions workflow so that it can exit if there are any warnings:

warnCount="$(echo $yarnoutput | grep -c -i "warn")"

if [ $warnCount != "0" ]; then
  echo -e "ERROR: Halting workflow because a warning was detected while installing..."
  exit 1
fi;

Synchronize NPM Tags

This action was created to clean up some of the tags on the NPM registry. It's supposed to trigger whenever a git branch is deleted. It would cross reference all of the branch names on Github with all of the tags on NPM and delete the ones that do not match. It had an optional "preserve" argument to keep some of the tags that may or may not have corresponding branches such as dev, beta, and alpha.

I don't know why I didn't have it just delete the corresponding tag of the deleted branch. But anyway, it looks like this action is being used in effection and microstates. We probably should've been using this for bigtest as well seeing as how we have a huge number of stale tags on NPM for bigtest packages.

Synchronize With NPM

This action is what we run in our release workflows. Basically it runs the npm publish command if the version of each package in the repository does not exist on the registry yet. Because we've been using changesets with this action, all the PR merges to the main branch do not get published since we rely on changesets to bump the versions and create its Version Packages PR.

git_setup()
find_packages()
  create a list of all the directories that contain package.json
  remove from the list the default ignore directories and any that are configured in the github workflow
  remove if package is private
  remove if package@version already exists on registry
publish()
  set npm token and unsafe-perm=true in .npmrc (token received as argument from github workflow) *
  run yarn or npm install depending on whether or not `yarn.lock` is present
  run custom command if specified in workflow
    this is for running any prepack or other necessary build commands
  for each directory in the list from the `find_packages()` function
    cd into that directory
    publish package
    git tag package_name-v$version
    git push origin package_name-v$version
      this tag format is to capture all the packages in the monorepo: @bigtest/webdriver-v1.0.0
    cd back to root for next loop
deprecate()
  get all package.json
  loop over list and if deprecate property is set in package.json
    npm run deprecate package_name deprecate_description

* I remember there was a lot of struggle around where to put npmrc and how multiple npmrc files conflict with one another. The context and conclusion to this issue is explained in #47.

Publish PR Preview

check_prerequisites()
  confirm the action is being run in a pull request
  confirm the action is not being run from a forked repository *
  confirm the action is not being run from a branch called 'latest' because it will conflict with npm's tags
json_locater()
  git diff between base and head ref
  remove file names and return an array of only the directories from git diff
  for each directory
    if there is no package.json
      root ? add root to array : go up one directory and run loop again
    if there is package.json
      add directory to array
remove_packages_to_skip()
  remove directories from array from json_locater() from default list and workflow argument
publish()
  npm_tag = branch name (format by replacing _ with __ and / with _)
  create published.json to record all the packages and its tags (for generating pr comment)
  yarn.lock ? yarn install : npm install
  set npm token in .npmrc
  run custom command if specified in workflow (for prepack/build)
    bigtest has this configured as yarn prepack:all
  for each directory
    cd directory
    if more than one package.json exists in directory and subdirectories (excluding node_modules)
      skip this directory
    if package is set to private
      skip
    npm publish --tag npm_tag
      bigtest has this configured as yarn publish
    append to the published.json created earlier with the package name and version
    cd to root for next loop
  if published.json lists 0 packages
    cat "we did not publish any packages" >> dangerfile.js
  if published.json has 1 package
    cat "we published {package}@{version-sha}" (simple version) >> dangerfile.js
  if published.json has > 1 packages
    cat "we published the following packages {dropdown with the details}" >> dangerfile.js
run_danger()
  install danger and set path
  run danger

* On a call with Charles, he mentioned that perhaps we could explore options around how a user in a forked repository could generate a preview package. He also mentioned that we do not need to generate preview packages for every pull request and that using labels to trigger the build might be a good idea.

I merged a Pull Request in Microstates repo but the publish failed because of the following error.

The problem is that the merge did not include a PR in the title. The commit history looks like this,

https://github.com/microstates/microstates.js/runs/248444449

We'd like the ability for this to automatically bump the version when publishing.

At the moment, we have to bump the version manually which becomes very error-prone especially when publishing mono-repo packages that rely on others in the same repo.

Perhaps an option to allow for automatic bump & publish on merge to a master branch could be a path forward?

Context

From this pull request on bigtest, I was removing unused packages from the monorepo and the discussion of deprecating those packages was initiated.

But first to address some of the comments from that pull request:

@taras: should we add a mechanism to release action to somehow deprecate packages?

• Or we could create a separate action for it.

@cowboyd: what about adding a deprecated flag to the package.json

• Wouldn't this require us to keep the package we want to remove (or at least the package.json) on the repo for at least one cycle? tag deprecate flag and push => let action run to deprecate packages => remove package and push

Solution

Create a new action to deprecate packages.

Approach

• Rename synchronize-with-npm to release and create the new action as deprecate
• Have it run on push to master branch.
• The action would compile the name property of every package.json of its latest (presumably from a pull request merge) commit and the one prior.
• Compare the two list to see if any packages were removed and compile list of removed packages.
• Loop over the list and run npm deprecate.

Motivation

I'm in the process of rewriting all of our actions in Typescript in preparation of releasing v2.

TODOs

• I think we should also discuss the idea of renaming our actions - mainly synchronize-with-npm but might as well rename the other ones too:

  Old Name	New Name
  synchronize-with-npm	publish-releases
  synchronize-npm-tags	synchronize-tags?
  publish-pr-preview	publish-previews

• We have the yarn action which we created in order to fail a workflow if there are any warnings during yarn install. I don't think we need to keep that as a separate action. If we really wanted to incorporate that, we could do so within the other actions. Is it okay to omit this one from v2?
  • Delete yarn action from v2 and update table at root readme #70
• Push current main branch to v2 to prevent any conflicts to workflows that are calling actions from the main branch
• Change base branch of remaining PRs to v2
• Once all actions are refactored, update all frontside projects to use @v2 Do this on a need-to basis
• Update readmes to use v2 (the readmes are currently instructing to call the actions from main branch) #70
• Delete main branch
  • These repositories need to update their workflows to migrate from main to v2 and we'll be able to delete the main branch:
    • thefrontside/element-is-visible #5
    • thefrontside/javascript #50
    • thefrontside/performance-api #6
    • thefrontside/microstates #426
      • If you scroll through here you should see they've all been updated (if the four PRs above have been merged)

We noticed that builds have been failing in https://github.com/thefrontside/backstage/pull/50

After some debugging, it turned out that this is called by a dependency that is not available in the registry. Here is how it happens, Package Versions PR contains versions in dependencies for package versions that haven't been published yet. When we run publish of previews, it fails because the dependant versions haven't been published yet.

This highlights 2 problems,

1. We should be updating dependencies with versions using preview versions that we're publishing.
2. We should be publishing in order dependencies before dependents

We encountered an issue trying to publish graphql-voyager to the organization's package registry.

It's a single-repo (as opposed to a monorepo), however it contains an example directory which has a package.json file in it.

Current Behaviour

The transparent publishing script assumes that this single-repo is a monorepo, due to the sub directory package.json files. Because of this it does not allow the root package to be published.

Desired Behaviour

The script should give the option for the root package to be published, despite the presence of sub-packages.

In the GraphQL Voyager case this would allow publishing of the root package. The example packages could also be published by setting private: false in their package.json.

This would also give monorepo's the ability to publish their root package. This is not typically desired, however giving the option to do so does not seem to be particularly harmful.

For some reason, some of our PRs don't publish previews. For example, I would expect thefrontside/playhouse#103 to publish a preview of @frontside/backstage-plugin-humanitec-common but it didn't. It's effecting our ability to developer experiment with these packages. Another example is thefrontside/playhouse#112 which never published a preview