feat(cli): add @ngxs/cli for generate store #520

kyusupov33 · 2018-07-24T15:26:05Z

closes #497

PR Checklist
Please check if your PR fulfills the following requirements:

The commit message follows our guidelines:
https://ngxs.gitbook.io/ngxs/recipes/style-guide
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes

What is the current behavior?

Issue Number: #497

What is the new behavior?

Now when you install @ngxs/cli it can helps to generate store structure on filled or empty format.
To do this, use the commands presented on the gif.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

If you want to verify that its work you need to:

checkout current branch, then

$ cd packages/cli
$ npm install
$ npm bin -g ngxs
$ npm link

You can try to use ngxs command in any directory and create store.

Contributors:

@kyusupov33
@splincode

markwhitfeld

Looks good to me...
Angular schematics would be first prize but this offers a great experience. We'll done!

@kyusupov33 Is it possible to have any tests for this?

kyusupov33 · 2018-07-24T16:25:50Z

@markwhitfeld We don't create tests for current generator, because usage plop (micro-generator framework that makes it easy to create files with a level of uniformity)
https://github.com/amwmedia/plop

markwhitfeld · 2018-07-25T10:54:49Z

@splincode Yes, please do.
Could you add the demo gif to the repo and use that one from the readme and docs (as opposed to the external link)?
Also, do you think that the command should be just 'ngxs' or should it be 'ngxs generate state' (similar to the angular cli)? it would allow for other usages of the ngxs cli.... maybe generate state is a reasonable default for no params...

@amcdnl Are you on board with the creation of a cli?

markwhitfeld · 2018-07-25T10:55:30Z

packages/cli/README.md

+? Write store name, please... 
+```
+
+![](https://habrastorage.org/webt/g5/td/dz/g5tddzflkkwftfvpvim3ckwveau.gif)


Should rather move the gif locally and use that one as opposed to the external link.

Agreed. Just add it to the repo.

amcdnl · 2018-07-25T13:45:09Z

First off, THIS IS AMAZING!

I was thinking about adding schematics but its kinda limited in what you can do. Ever since I noticed the Akita(?) CLI i was thinking about adding this to NGXS. I'll be reviewing this and try to cut a release this week!

amcdnl

Few nits.

amcdnl · 2018-07-25T13:45:36Z

packages/cli/README.md

+? Write store name, please... 
+```
+
+![](https://habrastorage.org/webt/g5/td/dz/g5tddzflkkwftfvpvim3ckwveau.gif)


Agreed. Just add it to the repo.

amcdnl · 2018-07-25T13:46:11Z

packages/cli/plopfile.js

+      {
+        type: 'input',
+        name: 'name',
+        message: 'Write store name, please...'


How about just Store name: the ... makes me think I'm waiting on something.

amcdnl · 2018-07-25T13:47:04Z

packages/cli/plopfile.js

+      {
+        type: 'directory',
+        name: 'directory',
+        message: 'Choose a directory..',


How about just Directory:. Again the ... makes me think I'm waiting on something. Also would be good to say (Default CWD)

amcdnl · 2018-07-25T13:47:40Z

packages/cli/plopfile.js

+      {
+        type: 'list',
+        name: 'storeType',
+        choices: ['Empty Store', 'Filled Store'],


At first I was confused what empty vs filled is. Would be nice to help clarify that.

This means that the user will have a choice:

empty store (skeleton)

A story filled with some data to learn how it works, for example

In the future, we can add a mini template for authorization, for example, both on the site. This will help a person who just starts using ngxs to look at it right away in the work, rather than studying the documentation. Perhaps it will be useful.

@amcdnl if you want we can delete filled templates and leave only empty. In the future we want to add example templates like: todos, auth, etc.
It will be look like this:

$ ngxs # Whitch store do you need? 1. Empty 2. Examples

If user choose 'Empty' store, then

# Store name: app # Directory: src

If user choose 'Examples' store, then

# Select example: 1. Generate todos store 2. Generate auth store # Directory: src

Examples store will help people to explore how ngxs is working.

@amcdnl remove it

amcdnl · 2018-07-25T13:48:22Z

Ya, I agree tests would be nice too!

kyusupov33 · 2018-07-25T14:15:36Z

About command names ... you're right. In the next commit, we'll fix it.

markwhitfeld · 2018-07-29T18:42:04Z

Wow! Nicely done!

markwhitfeld

A few tweaks and one template rework ;-)

markwhitfeld · 2018-07-29T18:43:20Z

docs/plugins/cli.md

+![CLI Screenshot](../assets/cli.gif)
+
+## Install
+The Devtools plugin can be installed using NPM:


Should be "The CLI can be installed ..."

markwhitfeld · 2018-07-29T18:44:38Z

packages/cli/README.md

@@ -0,0 +1,2 @@
+# @ngxs/cli
+Logger plugin for NGXS. See [repo](https://github.com/ngxs/store) for more info.


Should be "NGXS CLI" ...

markwhitfeld · 2018-07-29T18:47:46Z

packages/cli/src/templates/state.tpl

+})
+export class {{pascalCase name}}State {
+  @Action({{pascalCase name}}Action)
+  add(ctx: StateContext<{{pascalCase name}}StateModel>, action) {


Add type for action:
action: {{pascalCase name}}Action

markwhitfeld · 2018-07-29T18:52:32Z

packages/cli/src/templates/spec.tpl

+    addItem() {
+      this.store.dispatch(new {{pascalCase name}}Action('item-1'));
+    }
+  }


I think that it would be better to test the state class through the store directly and not via a component because component tests are quite expensive and are actually adding no real value here other than as a container for the store.

This spec file should be an example of how we recommend testing the state. Look here for inspiration:
https://ngxs.gitbook.io/ngxs/recipes/unit-testing

markwhitfeld

Amazing work. Just a few build pipeline issues to fix

markwhitfeld

Use jasmine instead of Mocha and for your tests if possible.

markwhitfeld · 2018-08-02T20:15:08Z

packages/cli/tests/test.ts

+import * as path from 'path';
+import * as rimraf from 'rimraf';
+import * as nodePlop from 'node-plop';
+import { expect } from 'chai';


If possible, please could you use jasmine as opposed to chai and mocha for these tests?

because Karma (Jasmine) does not work with node environment, we usage ts-mocha for testing CLI

angular/angular-cli#8357

node-plop usage child_process

plopjs/node-plop#81

markwhitfeld

Small tweak regarding command line param casing.

markwhitfeld · 2018-08-03T07:32:06Z

docs/plugins/cli.md

-  --folderName name   Use your own folder name, default: state  --spec boolean      Creates a spec file for store, default: true
+  --name name         Store name  
+  --directory path    By default, the prompt is set to the current directory
+  --folderName name   Use your own folder name, default: state  


@splincode @kyusupov33
Shouldn't the --folderName (camel case) argument rather be --folder-name (snake case I think it is called)?
I think that makes it have better support and less ambiguity across operating systems... or at least follows the broader convention.
The ng cli has the following syntax for example (from our package.json):
ng test --watch=false --code-coverage --progress=false --project ngxs

PS. Will have to update the cli demo gif to match the renamed param too ;-)

markwhitfeld · 2018-08-03T07:41:35Z

packages/cli/tests/test.ts

-    ).to.equal(actionsOutput);
+    const actionAppSnapshot = fs.readFileSync(path.resolve(expectedFiles.actions)).toString();
+    const actionsAppOutputCli = fs.readFileSync(path.resolve(generatedFiles.actions)).toString();
+    expect(actionAppSnapshot).to.equal(actionsAppOutputCli);


I like this, makes the tests much cleaner!

markwhitfeld · 2018-08-03T07:43:54Z

PS. Thanks for all the work you are putting into this @splincode & @kyusupov33!

splincode · 2018-08-03T09:45:48Z

splincode · 2018-08-03T10:55:01Z

@markwhitfeld @amcdnl I refactored the code, corrected the tests, corrected the documentation (screenshot)

markwhitfeld · 2018-08-03T12:56:11Z

docs/plugins/cli.md

  $ ngxs --help

 Options

  --name name         Store name  
  --directory path    By default, the prompt is set to the current directory
-  --folderName name   Use your own folder name, default: state  
+  --folde-name name   Use your own folder name, default: state  


Small spelling mistake ;-)

markwhitfeld · 2018-08-03T16:26:06Z

@amcdnl This looks like it is good to go in my books. I'm not familiar with the release procedure. Will this go out automatically when it is merged?
Also, do we want to keep the version numbers matching?

kyusupov33 · 2018-08-03T16:46:20Z

@markwhitfeld and @amcdnl
Thanks for the reviews, we tried to make cli as efficient as possible for use

splincode · 2018-08-05T18:25:18Z

@amcdnl
I wanted to demonstrate how CLI works
I rewritten everything into to typescript:
https://github.com/Angular-RU/ngxs-cli

$ npm i ngxs-cli -g
$ ngxs

Can you create a separate repository and we will move the changes there?
So that we could transfer everything there
https://github.com/ngxs/cli

amcdnl · 2018-08-07T14:22:58Z

Merged! I might move this into its own repo since its not really tied to any core pieces.

splincode · 2018-08-07T14:47:10Z

@amcdnl The problem is that they hurried to publish the project

Not found 404
https://www.npmjs.com/package/@ngxs/cli

Could you add me to the community, and give me the right to write?
https://github.com/ngxs/cli

Because the working project needs to be moved from here:
https://github.com/Angular-RU/ngxs-cli

markwhitfeld approved these changes Jul 24, 2018

View reviewed changes

markwhitfeld requested a review from amcdnl July 24, 2018 16:16

markwhitfeld reviewed Jul 25, 2018

View reviewed changes

amcdnl requested changes Jul 25, 2018

View reviewed changes

markwhitfeld mentioned this pull request Jul 29, 2018

docs: add pull request template #530

Merged

3 tasks

markwhitfeld requested changes Jul 29, 2018

View reviewed changes

markwhitfeld approved these changes Aug 2, 2018

View reviewed changes

markwhitfeld requested changes Aug 2, 2018

View reviewed changes

markwhitfeld requested changes Aug 3, 2018

View reviewed changes

markwhitfeld approved these changes Aug 3, 2018

View reviewed changes

markwhitfeld requested changes Aug 3, 2018

View reviewed changes

markwhitfeld approved these changes Aug 3, 2018

View reviewed changes

feat(cli): add @ngxs/cli for generate store

fa70a17

amcdnl merged commit 9dd8720 into ngxs:master Aug 7, 2018

		@@ -0,0 +1,2 @@
		# @ngxs/cli
		Logger plugin for NGXS. See [repo](https://github.com/ngxs/store) for more info.

feat(cli): add @ngxs/cli for generate store #520

feat(cli): add @ngxs/cli for generate store #520

Conversation

kyusupov33 commented Jul 24, 2018 • edited by markwhitfeld Loading

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Contributors:

markwhitfeld left a comment

Choose a reason for hiding this comment

kyusupov33 commented Jul 24, 2018

markwhitfeld commented Jul 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcdnl commented Jul 25, 2018

amcdnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcdnl commented Jul 25, 2018

kyusupov33 commented Jul 25, 2018

markwhitfeld commented Jul 29, 2018

markwhitfeld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markwhitfeld left a comment

Choose a reason for hiding this comment

markwhitfeld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markwhitfeld left a comment

Choose a reason for hiding this comment

markwhitfeld Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markwhitfeld commented Aug 3, 2018

splincode commented Aug 3, 2018

splincode commented Aug 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markwhitfeld commented Aug 3, 2018

kyusupov33 commented Aug 3, 2018 • edited Loading

splincode commented Aug 5, 2018

amcdnl commented Aug 7, 2018

splincode commented Aug 7, 2018

kyusupov33 commented Jul 24, 2018 •

edited by markwhitfeld

Loading

markwhitfeld Aug 3, 2018 •

edited

Loading

kyusupov33 commented Aug 3, 2018 •

edited

Loading