Loopback-next: [CLI] Get rid of Yeoman

Created on 5 Jan 2018  路  16Comments  路  Source: strongloop/loopback-next

Refactor the CLI package to use inquirer and ejs directly, get rid of the dependency on Yeoman.

This will allow us to use TypeScript too, we should migrate the CLI codebase to the standard TypeScript project as part of this task.

UPDATE 2020-02-11: Let's not limit ourselves to inquirer and ejs only. The ecosystem offers many more alternatives for building CLI tools, we should research the options and pick the best one. See the comments below for inspiration.

Related:

  • https://github.com/strongloop/generator-loopback/issues/139
  • https://github.com/strongloop/loopback-next/pull/825/files#r159253215
  • https://github.com/strongloop/loopback-next/pull/1384#issuecomment-401422692

List of issues compiled from the discussion:

  • In the "old" loopback-cli, our "model" generator is invoking "property" generator in a loop. This is not supported by Yeoman anymore, we have a hacky workaround in place. As a result, upgrades to newer versions of Yeoman were usually painful, because the hacky workaround had to be updated to match the new internals of Yeoman.
  • Invoking a generator directly from CLI is a bit of PITA, there are more hacks needed to make this happen - see e.g. https://github.com/strongloop/loopback-next/blob/6cc83b666e4b2ce2f979eab44dec78b7c01c41be/packages/cli/bin/cli.js#L66-L68
  • I find individual generators difficult to test in isolation. Verifying how CLI arguments are handled is even more difficult.
  • AFAICT, there is no way how to test non-trivial prompting logic - for example the loop asking for multiple properties. A mocked prompt can define only a single set of answers. Running the generator/CLI end-to-end and parsing stdout is brittle because Inquirer uses lots of ANSI control codes to control what's displayed on the screen.
  • Another missing feature is inability to verify configuration of individual prompts. For example when using a choices question populated dynamically (e.g. the datasource prompt showing all datasources known by the app), we want to verify the list of choices offered.
  • I also find the necessity to pepper our generator actions with if (this.shouldExit()) return false; as a horrible code smell. (See e.g. here and here and here). I was took a quick glance at other generators, not all actions start with shouldExit check, so we may already have subtle bugs there!
  • Subjectively, I dislike the conventions forced by Yeoman:

    • All generators live in different index.js files, which makes navigation difficult to me. Instead of app/index.js, I'd like to have app.generator.js file.

    • Implementing individual steps as named methods on a generator object made sense back in ES5 days, because it simplified async flow control. Today with ES2017's async/await, I prefer to write idiomatic JavaScript instead.

  • Reading from stdin has to use readline or complex workarounds, it's not possible to read entire input into a string using the usual streams API.
  • We need to call process.chdir in our tests, which makes it impossible to run the tests in parallel via worker threads. See https://github.com/mochajs/mocha/issues/2839 (We found a solution for this.)
CLI developer-experience major refactor

Most helpful comment

I suggest that we don't rush into a conclusion. It's probably the best choice to have a time-boxed spike to:

  1. Verify the pain points that @bajtos has listed and see there are fixes/solutions without abandoning Yeoman.

  2. Identify what's needed to build our CLIs using inquirer and ejs directly.

  3. Build a PoC for a simple generator such as app.

All 16 comments

@bajtos I would like to help however I can on this issue.

@raymondfeng Here are the issues I was experiencing in the old generator-loopback frequently:

  • Our "model" generator is invoking "property" generator in a loop. This is not supported by Yeoman anymore, we have a hacky workaround in place. As a result, upgrades to newer versions of Yeoman were usually painful, because the hacky workaround had to be updated to match the new internals of Yeoman.
  • Invoking a generator directly from CLI is a bit of PITA, there are more hacks needed to make this happen - see e.g. https://github.com/strongloop/loopback-next/blob/6cc83b666e4b2ce2f979eab44dec78b7c01c41be/packages/cli/bin/cli.js#L66-L68
  • I find individual generators difficult to test in isolation. Verifying how CLI arguments are handled are even more difficult.
  • AFAICT, there is no way how to test non-trivial prompting logic - for example the loop asking for multiple properties. A mocked prompt can define only a single set of answers. Running the generator/CLI end-to-end and parsing stdout is brittle because Inquirer uses lots of ANSI control codes to control what's displayed on the screen.
  • Subjectively, I dislike the conventions forced by Yeoman:

    • All generators live in different index.js files, which makes navigation difficult to me. Instead of app/index.js, I'd like to have app.js file.

    • Implementing individual steps as named methods on a generator object made sense back in ES5 days, because it simplified async flow control. Today with ES2017's async/await, I prefer to write idiomatic JavaScript instead.

@privateOmega thank you for chiming in! Right now, we are debating whether we actually want to drop Yeoman or whether it's perhaps not that bad. Once we reach consensus, I can show you pointers where to start.

AFAICT, there is no way how to test non-trivial prompting logic - for example the loop asking for multiple properties. A mocked prompt can define only a single set of answers. Running the generator/CLI end-to-end and parsing stdout is brittle because Inquirer uses lots of ANSI control codes to control what's displayed on the screen.

Another missing feature is inability to verify configuration of individual prompts. For example when using a choices question populated dynamically (e.g. the datasource prompt showing all datasources known by the app), we want to verify the list of choices offered.

I also find the necessity to pepper our generator actions with if (this.shouldExit()) return false; as a horrible code smell. (See e.g. here and here and here). I was took a quick glance at other generators, not all actions start with shouldExit check, so we may already have subtle bugs there!

I feel like this is something that should be done ASAP before we pile on more features onto our CLI tool. I'm for replacing Yeoman if we can come up with a prototype that addresses the problems that Yeoman faces.

I suggest that we don't rush into a conclusion. It's probably the best choice to have a time-boxed spike to:

  1. Verify the pain points that @bajtos has listed and see there are fixes/solutions without abandoning Yeoman.

  2. Identify what's needed to build our CLIs using inquirer and ejs directly.

  3. Build a PoC for a simple generator such as app.

+1 for starting with a spike.

+1 for spike. Aside from verifying the pain points listed above and the plan outlined by @raymondfeng, are there any additional questions we want to answer in this spike?

With the spike #1187 created, I'm not sure what's the best way to handle this issue. Keep it open? change it to be an epic?

An issue that arose from #1578:

  • when error is thrown by throw new Error(), error is both rejected AND emitted. yeoman-test module does not take this into account which leaves us with two rejected promises to deal with.

A module we could use for interactive prompts: https://github.com/terkelg/prompts

I especially like autocomplete prompt, I think it will give much better UX for large projects.

https://oclif.io seems to be a popular library used e.g. by Heroku CLI.

https://oclif.io seems to be a popular library used e.g. by Heroku CLI.

Looks very good. +1 for plugins @oclif/plugin-not-found (e.g. "did you mean?") @oclif/plugin-autocomplete (add bash autocomplete).

Added another pain point:

We need to call process.chdir in our tests, which makes it impossible to run the tests in parallel via worker threads. See https://github.com/mochajs/mocha/issues/2839

Was this page helpful?
0 / 5 - 0 ratings