Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to play well with Yeoman's run loop #139

Closed
bajtos opened this issue Jan 30, 2016 · 4 comments
Closed

Refactor to play well with Yeoman's run loop #139

bajtos opened this issue Jan 30, 2016 · 4 comments

Comments

@bajtos
Copy link
Member

bajtos commented Jan 30, 2016

A follow-up for #116 (comment).

We are using yeoman generators in a way that was supported in pre-v0.17 days, but that is different from the new direction where yeoman is heading nowadays.

Our design is based on assumption that generators work like javascript methods: if we want to compose "loopback:model" with "loopback:property", we can simply generator#invoke() the later and the execution of the former would be postponed until the nested generator finishes.

However, that's no longer the case. These days, Yeoman uses a run queue with named groups (see "The Run Loop" in docs). When two generators are composed, their steps/actions are interleaved together via these named groups (e.g. "prompting", "configuring", "writing", "install", etc.). This composition does not allow repeated invocation of a nested generator, as we are doing now.

I think we should rework generator-loopback, refactor most of the prompts & actions so that they don't depend on yeoman features and the yeoman integration becomes just a thin wrapper. As a side effect, it should become easier to write unit-tests for prompts to verify input validation for example.

Mockup for illustration:

// model generator
generators.Base.extend({
  prompting: {
     promptForModelData: function() { 
       this.modelData = prompts.modelData();
     },
     promptForProperties: function() {
       this.properties = [];
       for (;;) {
         var prop = prompts.propertyData(this.modelData.name);
         if (!prop.name) break;
         this.properties.push(prop);
      }
  },
  writing: {
     createModel: function() {
       scaffold.model(this.modelData);
     },
     createProperties: function() {
       this.properties.forEach(function(prop) {
          scaffold.property(prop);
       });
     }
  }
});
@bajtos
Copy link
Member Author

bajtos commented May 31, 2016

This should also allow us to write automated tests that would prevent a situation that happened recently, when we modified prompt validation to return a promise because of a breaking change in yeoman, and then when yeoman published a new release fixing that breaking change, nobody noticed that generator-loopback no longer works, because CI was happily passing.

@hacksparrow
Copy link
Member

@bajtos why can't we do without Yeoman?

@bajtos
Copy link
Member Author

bajtos commented Feb 16, 2017

why can't we do without Yeoman?

At the I opened this issue, we were relying on Yeoman's yo command to run our generators (commands). It also seemed like a good idea to play well with Yeoman ecosystem.

Things has changed since then a lot. Among other things, we have https://github.com/strongloop/loopback-cli as the top-level CLI that LoopBack users should use instead of yo.

IMO, now we can drop Yeoman completely and rewrite loopback-cli using Inquirer + loopback-workspace only.

@dhmlau
Copy link
Member

dhmlau commented Jul 23, 2019

Closing due to inactivity. This module will only be used for LB3.

@dhmlau dhmlau closed this as completed Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants