RFC 5 - Split smart behaviour out of version and publish commands

[digitalsadhu]

Description

Currently the eik version command does its best to detect if there have been any changes to the code to be published and bails from versioning if there haven't. This PR suggests that this sort of behaviour most appropriate for the CI context and therefore should be changed slightly.

The current usual workflow is:

eik version [major|minor|patch]
eik publish

And this will work so long as there are changes to publish. When working locally, you usually know what version you want and when you want to publish it and do not require the smarts built in to detect version changes. When on CI however, you specifically want to avoid the situation where every single time there is a new push to master, a new Eik version is published regardless of whether any code actually changed.

Additionally, when publishing, if the publish fails because the current version of the package is already published, locally its appropriate for this to error and give you an error message. On CI, this breaks the build when preferably it would log the error for your information and then silently move on so that the build can succeed.

Proposed solutions

Add a --ci flag to the version and publish commands

One option is to make the version and publish commands dumb by default and smart for ci when a cli flag is supplied --ci

For example:

eik version  --ci
eik publish --ci

Add a new command eik ci

An alternative is to make the version and publish commands dumb by default and add a new ci specific command eik ci

For example:

eik ci

My preference is for the latter but would like to hear any thoughts on this

[gingermusketeer]

Like the idea of breaking this up so that it is easier to understand, but I find that the CI flag doesn't really communicate very much.

What about adding a check command that sees if something needs to be done?

On CI the command could then become something like eik check || eik version && eik publish?

[digitalsadhu]

Monorepos are challenging to support with semantic release due to the potential for multiple builds running in parallel to try to push back to the same repo. First build to push wins and later builds receive an error that the GH head has moved.

It's easy enough to avoid this issue with the following workflow:

1. Run eik version locally (detects if new version is needed, bumps if yes and commits the result)
2. Push changes
3. eik publish runs on CI (publishes if there is a new version)

The main issue with this is that the user needs to remember to run eik version locally. This can be probably be solved by just adding a lint-staged type pre commit hook to run version whenever you commit your files. (untested as of yet).

All that as background for making the case that a way to run eik publish on every build run regardless of whether a new version has been published would be ideal. This is being done already at Finn by running something like:

eik publish || true

To silently ignore errors. This works but its a bit fugly and comes with the disadvantage that ALL errors from eik publish will be silently ignored when ideally we ONLY want publishes due to no new version to be silently ignore.

I'm beginning to think that eik publish should be changed to automatically ignore failed publishes if those publishes are not the result of an actual error. Ie. it's not considered an error if a publish was just not needed due to an already existing version. If necessary a --strict flag could be added to fail on everything.

Something like

# only fails on unexpected errors
eik publish

# fails when requirements are not met (such as an unpublished version number)
eik publish --strict

Thoughts @trygve-lie ?

[digitalsadhu]

Another point though is that it might be nice to add the check command as @gingermusketeer suggested that could be run either locally from a hook (like off of an npm test) or from CI for those that don't mind bumping the version manually but would like to be nicely informed if they forget to do so

{
  "test": "tap",
  "posttest": "eik check"
}

[trygve-lie]

I am in favor of the idea of not trying to be smart (detecting changes) in one setting and trying to be smart in another. I'm also tinting towards adding a eik cicommand for this over using an argument.

On the topic of monorepos; what if eik had understanding of monorepos? What if the eik config had a workspaces features down the same line as in npm?

I'm thinking something down this line; in a monorepo one operate with a eik.json which more or less only holds an array of paths to packages:

{
  "name": "my-monorepo-project",
  "workspaces": [
    "app-a",
    "app-b"
  ]
}

This would make it possible to be in root of a monorepo and run the new eik ci command once but the command will run on each workspace and just publish those which has changes in them. Would such an approach benefit the semantic release process too?

[renatewr]

I like the idea of "workspaces", but I think a "workspaces": true is sufficient, and then read root package.json on root to know where/what those packages are, with an option to add a list in eik.json if you prefer to keep that list manually updated. I would prefer not having to maintain that list manually..

package.json

"workspaces": [ "packages/*" ],

I can tell you how I have solved it it our monorepo designsystem, using independent versioning and publishing to Eik the version in package.json for each package.

Every "package" has a the following (or similar) in their package.json:

"eik": { "server": "https://assets.acdn.no", "files": "./dist/eik" },

We are using changesets to automate versioning and changelogs. Whenever a new release is happening, the output is a list of successfully published packages.

Thus, I created a custom workflow action that takes that array as an input.

So, input is an array of objects with package names (similar to what a workspace list would do).

[{"name":"@amedia/brick-teaser", "version":"0.0.0"}]

The "version" is not relevant, as it actually uses the version in package.json, that has been updated by the previous release job.

Here is the code, which was a POC and has its flaws. Please comment if you have improvements.

I would like the output/error handling to be better, and also be able to use the output to post to slack the result. Right now, if it fails, it fails silently and we need to go look in the logs if its ok or not, since the build succeed.

export async function getVersionsByDirectory(cwd, validPackages) {
  const { packages } = await getPackages(cwd);
  if (!Array.isArray(validPackages)) {
    console.log(typeof validPackages);
  }
  const pkgList = {
    validPackages,
  };
  // it should filter out packages without eik config
  const filteredPackages = packages
    .filter((pkgObj) =>
      pkgList.validPackages.some(
        (criteria) =>
          criteria[Object.keys(criteria)[0]] ===
          pkgObj.packageJson[Object.keys(criteria)[0]]
      )
    )
    .filter((p) => 'eik' in p.packageJson); // filter out non-eik packages
  return new Map(filteredPackages.map((x) => [x.dir, x.packageJson.version]));
}

export async function execWithOutput(command, args, options) {
  let myOutput = '';
  let myError = '';

  return {
    code: await exec(command, args, {
      listeners: {
        stdout: (data) => {
          myOutput += data.toString();
        },
        stderr: (data) => {
          myError += data.toString();
        },
      },

      ...options,
    }),
    stdout: myOutput,
    stderr: myError,
  };
}

export async function execEik(version, dir) {
  console.log("Exec process chdir to dir: ${path.join(dir)}");
  process.chdir(path.join(dir));

  let myOutput = '';
  let myError = '';

  const options = {};
  options.listeners = {
    stdout: (data) => {
      myOutput += data.toString();
    },
    stderr: (data) => {
      myError += data.toString();
    },
  };
  options.cwd = "${path.join(dir)}";
  const task = await exec('npx', ['eik', 'package'], options)
    .then(() => exec('npx', ['eik', 'package-alias'], options))
    .catch((e) => console.log('exec eik package', e));

  return { task, stdout: myOutput, sterr: myError };
}

export async function runEikVersion({ list }) {
  console.log("List of packages to run eik package on: ${[list]}");
  const results = [];
  list.forEach((version, dir) => {
    results.push(execEik(version, dir));
  });

  const contents = await Promise.all(results.map((p) => p.catch((e) => e)));
  const validResults = contents.filter((result) => !(result instanceof Error));
  const erroredResults = contents.filter((result) => result instanceof Error);
  return { validResults, erroredResults };
}