Expose Frontity commands to be used programmatically

[luisherranz]

— Roadmap Card
— Sprint Card

Description

We need to refactor the way our commands work so the steps are more easily reused and developers can access the commands programmatically.

User Stories

As an Integration Engineer
I want to use the Frontity commands programmatically
so that I can integrate Frontity in my CI/hosting/deploy tools.

— Pull Request

As an Integration Engineer
I want to use env variables to configure Frontity commands
so that I can integrate Frontity in my CI/hosting/deploy tools

As a core developer
I want to resuse steps in different commands
so that we don’t have to maintain duplicate code

As a core developer
I want to use commands in different interfaces
so that we can release more than one interface

Proposed implementation

• Logic is isolated in steps, which are the smaller unit of code.
• Multiple steps form a command.
• Commands can be called directly or by interfaces.

Steps -> Commands -> Interfaces

Steps

Steps are small functions that isolate logic that can be used by one or more commands. They should do only one task. They can return either the result of that task or a promise that resolves to the result in case the task is asynchronous. They throw an error if something goes wrong.

Some examples:

• isDirEmpty
• ensureDirExists
• createPackageJson
• createFrontitySettings
• cloneNpmPackage
• installDependencies
• downloadFavicon
• createIndexJs
• subscribeToNewsletter

Each step should receive the arguments it needs to execute the task. It should not receive the whole options of the command, because can be used by more than one command.

export const downloadFavicon = async (path: string) => {
  const response = await fetch(faviconUrl);
  const fileStream = createWriteStream(resolvePath(path, "favicon.ico"));
  response.body.pipe(fileStream);
  await new Promise(resolve => fileStream.on("finish", resolve));
};

Commands

Commands wrap more steps together and can be used directly or by interfaces.

This is the list of the current commands:

• dev
• build
• serve
• create
• create-package
• subscribe
• info

These are the commands we want to add in the short term:

• deploy
• install-package
• rename-package
• update-package

They will be exported in frontity/commands (that means we have to rename the current commands.ts file to cli.ts).

Commands are async functions that resolve when all the steps are finished. They receive an options object as first parameter.

import { create, build } from "frontity/commands";

await create({
  name: "my-frontity-project",
  starter: "@frontity/mars-theme",
  typescript: false
});

await build({
  buildFolder: "/public",
  target: "module",
  mode: "production"
});

Parameters can be optional and need a default value.

When some parameter is not included, the command will check for an environment variable with the same name and the FRONTITY prefix. If it is not present, it will use the default.

// This build will use "/build" because it's the internal default
await build();

// This build will use "/public" because options take precedence.
process.env.FRONTITY_BUILD_FOLDER = "/dist";
await build({
  buildFolder: "/public",
});

// This build will use "/dist" because options is empty.
process.env.FRONTITY_BUILD_FOLDER = "/dist";
await build();

All commands reside in the frontity package except dev, build and serve which are in the @frontity/core package.

To reduce the size of frontity and avoid download unrelated packages when people use npx frontity create, the dependencies of all the steps used by the create command will be "dependencies" in the frontity package, and the rest will be "devDependencies" in frontity and "dependencies" in @frontity/core. That’s because once the Frontity project is created, @frontity/core is mandatory.

Commands need to be able to communicate their internal progress to the interfaces.

This is the part I’m not 100% sure about, but maybe we could include some type of callback that it is called when steps finish.

const buildFinished = await build(options, ({ awaiting, started, finished }) => {
   // Update React state.
  setAwaiting(remaining);
  setStarted(started);
  setFinished(finished);
  const total = awaiting.length + started.length + finished.length;
  setProgress(finished.length/total);
});

It’s important to note that a command can execute several steps in parallel.

If a step fails, they should bubble up the error. They may also have a mechanism to revert the changes they’ve done so far. For example: delete the folder they have created or revert the changes to a file.

Interfaces

Interfaces provide a way for users to gather the options needed for each command and then execute those commands.

Right now we have two in mind.

• Frontity CLI (current)
• Frontity Admin UI (coming)

The frontity package will include the CLI for now, but it’s good if we start thinking that the same commands will be used by the Admin UI as well.

Frontity CLI

The cli will prompt for the options of that command:

> npx frontity create
- Name of your project? "my-frontity-project"
- Which starter theme to use? "@frontity/mars-theme"
- Do you want to use TypeScript? Y/n

If an option is passed using arguments, it will be used to populate the placeholder:

> npx frontity create --starter @frontity/mars-theme
- Name of your project? "my-frontity-project"
- Which starter theme to use? # (@frontity/mars-theme)

Only in the case of the presence of the arg --no-prompt, the cli won’t prompt for any option and it will execute the command directly.

In that case, it will use the options provided by the arguments and it will leave the rest not defined so the command can use env variables if they are present.

> npx frontity create "my-project" --typescript --no-prompt
[/] Creating your project...

// This will run:
await create({
  name: "my-project",
  typescript: true
})

Command options object properties, cli params and env variable names must be easily inferrable from each other.

> npx frontity create --typescript

await create({
  typescript: true
});

process.env.FRONTITY_TYPESCRIPT = true;

> npx frontity build --build-folder public

await create({
  buildFolder: "public"
});

process.env.FRONTITY_BUILD_FOLDER = "public";

Next steps

This is a mixed list of issues and next features for reference. I’ll create new features for some of the items.

• Migrate all the steps to individual files and refactor commands and interface folders.
• Fix the create steps to be able to include more starter themes.
• Refactor commands to work with the new callback function.
• Add support for env variables.
• Migrate current commands to Ink.
• Add e2e testing (in mac, win and linux with both npm and yarn).
• Fix typescript in create.
• Improve create with additional questions about: git, eslint, prettier, gutenberg…

[mmczaplinski]

If a step fails, they should bubble up the error. They may also have a mechanism to revert the changes they’ve done so far. For example: delete the folder they have created or revert the changes to a file.

Agreed, this is probably quite important so that people don’t get weird surprising errors if something fails to proceed.

Commands need to be able to communicate their internal progress to the interfaces.

I think this is not necessary. For most commands it will be difficult or impossible to estimate the “true” progress e.g. because of network latencies. I think that it’s sufficient to provide the information on whether the particular action has started / finished.

This is the part I’m not 100% sure about, but maybe we could include some type of callback that it is called when steps finish.

const buildFinished = await build(options, ({ awaiting, started, finished }) => {
   // Update React state.
  setAwaiting(remaining);
  setStarted(started);
  setFinished(finished);
  const total = awaiting.length + started.length + finished.length;
  setProgress(finished.length/total);
});

This part is not very clear to me. I think we can accomplish the same goals using the “eventEmitter” pattern. What would be the benefit of doing passing the callback to build()?

[mmczaplinski]

@luisherranz

I have started the effort to refactor the CLI to use the structure that you outlined but I am not confident that in my ability to design the steps & commands in a way that is 100% independent of the interface without significant effort.

One way that I would imagine it is to focus exclusively on the “state” and “actions” necessary to run all the commands, without any UI. This could be modeled as a MobX store, redux tree, or I guess even using frontity/connect . Then, we could migrate the UI of the CLI to Ink and create the UI of the web version.

My concerns are:

1. All of this is not a small task and is almost a complete rewrite of the CLI. The tests would have to largely be rewritten too.
2. It doesn’t make sense to migrate to Ink in isolation. So, basically, we are BOTH refactoring, introducing new features (Ink renderer) and rewriting the tests which is many changes at once that are hard to separate and hard to keep regressions from creeping in.

Is it worth the time and effort now is the key question

[luisherranz]

Yes, because the CLI is still pretty small but it will get much more complex in the future and the same logic will be reused in more places. So we need to do this now.

Don’t mind about the ink port yet, let’s reorganize the logic in reusable steps and commands first.

And don’t worry about the callback/emiter either. We can omit that part if that’s difficult. It’s not important for the user experience.

[luisherranz]

Maybe we can use a combination of EventEmitter and Promise, like this library: https://www.npmjs.com/package/event-promised

[mmczaplinski]

It won’t work because you cannot do anything with the Promise that is being awaited other than simply awaiting it

So, this API is not going to work:

async function build(options) {
  return new EventPromised((resolve, error, emit) => {
    ...
    ... build command...
    ...
    emit("build:finished", "some message");
    resolve("Promise solved");
  })
}

const eventPromised = await build(options);
eventPromised.on('build:finished', msg => console.log(msg))

But, I think it’s okay if we solve it in a “stupid” way

We can just create a single eventEmitter in a separate module and then namespace the events for each command and interface.

import { EventEmitter } from "events";

type eventTypes = "cli:create" | "cli:create:error" | "cli:create:subscribe";
let emitter: EventEmitter;

export { emitter }; 

And then just use it wherever.
The package https://github.com/andywer/typed-emitter will also give us type safety / autocompletion for the event types that we define ourselves.

[luisherranz]

If you want to use both the event emitter and the promise you can delay the awaiting of a promise:

const buildPromise = build(options);
buildPromise.on('build:finished', msg => console.log(msg))
const buildResult = await buildPromise;

If you don’t want to use the event emitter, you just await the promise:

const buildResult = await build(options);

[mmczaplinski]

@luisherranz

If you want to use both the event emitter and the promise you can delay the awaiting of a promise:

const buildPromise = build(options);
buildPromise.on('build:finished', msg => console.log(msg))
const buildResult = await buildPromise;

Hmm, I don’t think this is going to work because whatever I return from build in your example is going to be automatically wrapped in a Promise. So, I can’t return the EventPromised object and call it’s .on method, because the EventPromised will be wrapped in a native promise.

problems with promises - CodeSandbox

[luisherranz]

Oh, but EventPromised is already a promise, you don’t need to wrap it on a promise again

This is the corrected example:

problems with promises - CodeSandbox

[luisherranz]

I’d also like to talk about the testing strategy we are going to take from now with steps, commands, and interfaces. Right now we only have unit tests for steps.

Steps

I think we should either:

• Add end to end tests and keep the unit tests.
• Delete the unit tests and only maintain e2e tests.

The problem with unit tests is that a lot of the steps are going to deal with stuff which is different in each operative system, like the file system for example. We need to have that covered, so I’d like to run the e2e tests in ubuntu, mac, and windows using GitHub actions.

We can create these tests with jest, but instead of mocking the libraries we check that the things that need to happen are actually happening. For example, something like this:

beforeEach(() => {
  rimfar("./tmp");
});

test("createPackageJson with some specific options", () => {
  const options = { path: "./tmp", /* more options... */ };
  await createPackageJson(options);
  const packageJson = await readFile("./tmp/package.json", "utf8");
  expect(packageJson).toMatchSnapshot();
});

Commands

For commands maybe it’ll be enough to do unit tests and assert that the correct steps have been called:

// Mocked steps
steps.createDir.mockReturnValue(Promise.resolve(true));
steps.createPackageJson.mockReturnValue(Promise.resolve(/* some value... */));

test("create with some specific options", () => {
  const options = { name: "my-app", typescript: false };
  await build(options);

  expect(steps.createDir).toHaveBeenCalledWith("my-app");
  expect(steps.createPackageJson).toHaveBeenCalledWith({ name: "my-app", /* ... */ });
  // More asserts...
});

Interfaces

I think we should not test the current interface because it’s going to be deprecated soon when we move everything to ink.

Once we move to ink, we can use the stdin.write method to simulate the keyboard. Then, check with mocked commands if it is calling them with the correct parameters.

For example, start the create interface with some args, simulate the keyboard entry and assert the commands:

commands.create.mockReturnValue(Promise.resolve(true));
commands.subscribe.mockReturnValue(Promise.resolve(true));

test("create cli with some options", () => {
  const args = [ "my-app",  "--git" ];
  const { stdin } = render(cli.create(args));
  stdin.write("\r"); // Accept name (prepopulated via args).
  stdin.write("N\r");  // Don't opt-in to TypeScript.
  stdin.write("\r"); // Accept git (prepopulated via args).
  stdin.write("@frontity/twentytwenty-theme\r"); // Starter theme.
  

  expect(commands.create).toHaveBeenCalledWith({
    name: "my-app",
    typescript: false,
    git: true,
    packages: [
      "@frontity/twentytwenty-theme"
    ]
  });

  stdin.write("Y\r"); // Subscribe to the newsletter.
  stdin.write("myself@mydomain.com\r"); // Enter email

  expect(commands.subscribe).toHaveBeenCalledWith({
    email: "myself@mydomain.com"
  });

---

Thoughts?

[mmczaplinski]

That works, you’re right, but it’s because we made the build command to not be an async function anymore.
I guess we can just wrap the body of the build command with the event emitter and make the callback async in that case. So, like:

function build() {
  return new EventPromised(async (resolve, error, emit) => {
    

    await createPackageJson(1000);
    emit('created package.json');

    await createFrontitySettings();
   ....
  });
}

But to be honest, I feel like the current architecture with the eventEmitter as a singleton in a separate module is not bad. We can namespace the events for each command. This is simply what I did:

import { EventEmitter } from "events";
import TypedEmitter from "typed-emitter";

interface MessageEvents {
  "cli:create:error": (error: Error) => void;
  "cli:create:subscribe": (msg: string, action: Promise<any>) => void;
  "cli:create:message": (msg: string, step: Promise<any>) => void;

  "cli:subscribe:message": (msg: string, action: Promise<any>) => void;
  "cli:subscribe:error": (error: Error) => void;
}

const emitter = new EventEmitter() as TypedEmitter<MessageEvents>;

export { emitter };

And then I can import the emitter in the create command.

For testing, I can mock the module with jest. Easy.

Let me know if there is some problem with this approach that I perhaps fail to see

[mmczaplinski]

Yeah, I mostly agree basically I think we can keep most of the unit tests that we already have. I made them work with the updated structure.

Also, we already have some tests for the commands, actually, that work in the way that you’ve described. I think they were done be Eduardo. They assert that the particular steps of the command have been called. For example:

   test("calls removeProgress on error with dirExisted=true", async () => {
   const options = {
      name: "random-name",
      path: "/path/to/project"
    };
    mockedSteps.ensureProjectDir.mockResolvedValueOnce(true);

    const error = new Error("Mocked Error");
    mockedSteps.createPackageJson.mockImplementation(() => {
      throw error;
    });

    await create(options);
    expect(emitter.emit).toHaveBeenLastCalledWith("cli:create:error", error);
    expect(mockedSteps.revertProgress).toHaveBeenCalledWith(true, options.path);
  });

About the tests for steps, I totally agree about also testing the tests for system differences, etc. This is probably gonna be one of main sources of pain. What I would also like to test for are things like:

• spaces in filenames, usernames, etc.
• network access disappearing, e.g. when downloading packages (so that we retry and timeout gracefully instead of waiting for a promise to resolve forever
• ctrl + c quitting the shell should clean up the directory
• correctly handling both case sensitive vs case insensitive filesystems.

About testing interfaces:
Not much to add, just

[luisherranz]

But to be honest, I feel like the current architecture with the eventEmitter as a singleton in a separate module is not bad.

Can you show me how it would be consumed by an external script or interface? Imagine for example I am a hosting and I want to control the build programmatically after a git push, and I want to show the progress of the build in my dashboard.

import { build } from "frontity/commands";

export const push = () => {
  try {
    await build();
    // => Show the build progress in the dashboard.
  } catch (error) {
    // => Show error in the dashboard.
  } 
};

Yeah, I mostly agree basically I think we can keep most of the unit tests that we already have. I made them work with the updated structure.

Awesome. Go ahead then

[mmczaplinski]

Can you show me how it would be consumed by an external script or interface? Imagine for example I am a hosting and I want to control the build programmatically after a git push, and I want to show the progress of the build in my dashboard.

I think you cannot easily

If that’s the requirement, I will just use the eventPromised instead

[mmczaplinski]

I think this should be the correct priority of sources of the values that we use in the CLI.
Could you check this @luisherranz ?

[luisherranz]

Yes, I think so too

The only thing I would add is, if there is no “ENV supplied” , I would only throw an error for required arguments (like name in frontity create) but I would use the default for the optional ones (like false for typescript in frontity create). That way people don’t need to provide all the args when they use --no-prompt.

Nice graphic by the way

[mmczaplinski]

Nice graphic by the way

haha, using https://excalidraw.com/ for this.

if there is no “ENV supplied” , I would only throw an error for required arguments

Yes, that’s what I had in mind exactly. Thanks for clarifying.

[mmczaplinski]

This has been implemented in https://github.com/frontity/frontity/pull/262

What has happened:

• Clean up and make the API more consistent. Now every command can be used programmatically like:

import { create, build } from "frontity/cli";

await create({
  name: "my-frontity-project",
  starter: "@frontity/mars-theme",
  typescript: false
});

• Use environmental variables in the commands if present according to:
  
  
• Lots of small improvements and refactoring