[legacy-framework] feat(cli): generate command #187

aem · 2020-04-22T21:39:49Z

Pull Request Type

Feature

Checklist

Tests added for changes (or N/A)
n/a Any added terminal logging uses packages/server/src/log.ts (or N/A)
- I thought it might be worth having a separate discussion about creating a separate logging package rather than having logging tied to the server package. For now I just reimplemented the minimal logging required within command.ts so all commands have access
Code Coverage stayed the same or increased

What's the reason for the change? ❓

Closes blitz-js/legacy-framework#862, closes blitz-js/legacy-framework#845

What are the changes and their implications? ⚙️

Introduces blitz generate. Here's the docs for blitz generate --help, which should be fairly self-explanatory as far as the implications of this PR:

Generate new files for your Blitz project

USAGE
  $ blitz generate TYPE NAME

ARGUMENTS
  TYPE  (all|crud|mutation|page|query|resource) Type of files to generate
  NAME  Namespace for generated files (e.g. [name]Query.ts).

OPTIONS
  -d, --dry-run        Show what files will be created without writing them to disk
  -h, --help           show CLI help

  -p, --parent=parent  The parent context for nested generation. For example, generating `products` within a `store` would supply `-c 
                       store`. For nested contexts you may supply the full path.

ALIASES
  $ blitz g

EXAMPLES
  # We can just generate a single file type, for example queries. In this case,
  # the generator will ask any questions necessary to generate the proper files.
  blitz generate query task
    
  # Or, we can generate a full set of models, mutations, pages, etc.
  blitz generate all task
    
  # We can specify where the files belong with the parent flag, this allows you
  # to generate nested routes automatically. The example below would generate a route
  # for https://myapp.com/taskManager/task.
  blitz generate page task -c=taskManager

The one thing left up for debate is the terminology of resource. In my mind I've been thinking about resource to mean a full set of files (model + mutation + page + query), but I can see how that can be confusing. For example, in the Java/REST world "resource" usually means a single REST endpoint or a small collection of related endpoints.

Having said that, I do think if our goal is to truly create a full app ecosystem, I think we can reasonably take some creative license here to define our own DSL. As long as our docs are clear and have good guides for these words (e.g. a glossary that defines what we mean when we say "resource") I think it's totally reasonable to redefine these words to have special meaning in the context of Blitz.

Does this introduce a breaking change? ⚠️

Yes
No

Other information

The generate command functions as a wizard and attempts to prevent you from hitting pitfalls as much as possible. You'll notice >100 lines of validation code in the command, the idea is to provide guardrails as much as possible to prevent users from doing anything bad, and either correcting for them or giving them a prompt to correct the error themselves rather than either (a) making assumptions that could be destructive, or (b) just failing hard.

flybayer · 2020-04-23T06:56:41Z

packages/cli/src/command.ts

+  protected indent = false
+
+  logTheme(str: string, options?: LogOptions) {
+    this.log(chalk.bgWhite.hex(this.themeColor).bold(str), options)


Can we reuse the log util for these? I've opened #194 for adding a utils package, but importing from server is fine for now.

ok that's fine, I can do that. I mentioned in the PR description that using server as a logger felt like a weird merging of responsibilities for that package so I held off initially. I'll move to server under the assumption that it'll eventually get codemodded to the utils package

Thanks! Better to make a step towards the right solution than no step at all :)

flybayer · 2020-04-23T06:57:31Z

packages/cli/src/commands/generate.ts

+
+enum ResourceType {
+  Resource = 'resource',
+  Model = 'model',


We don't currently have model files, so just leave this out for now

I was figuring that model would add the model to the prisma file, generate a query + mutation, and then run prisma migrate. Just a shorthand for adding data to the db without generating a UI for it

flybayer · 2020-04-23T06:59:32Z

Sweet! All the input checks are great!!

Over in the main issue I proposed blitz generate crud [context/]MODEL --parent PARENT_MODEL, which I like pretty well for defining context path and parent path. Sorry if you missed those comments since they aren't in the first post of the thread. See here: blitz-js/legacy-framework#862 and blitz-js/legacy-framework#862

Rails uses rails generate scaffold Context::User for context namespacing which matches the Ruby namespaced class syntax. So I think using Context/User fits nicely for us. We should support both \ and /

flybayer · 2020-04-23T07:17:46Z

Here's what I think we should use for naming. Rails and redwood use scaffold for generating everything. And rails uses resource for just model + controllers. So being consistent here is nice for folks if we don't have a good reason to deviate.

Fri. Alpha	Command	Result
Critical	`blitz generate crud`	All crud queries/mutations
X	`blitz generate resource`	All crud queries/mutations + model schema
Preferably	`blitz generate all`	All crud queries/mutations + pages + model schema
X	`blitz generate page`	A page
X	`blitz generate component`	A component
X	`blitz generate query`	A query
X	`blitz generate mutation`	A mutation

packages/cli/src/commands/generate.ts

flybayer · 2020-04-23T09:51:43Z

oops!

flybayer · 2020-04-23T09:52:21Z

@aem there's some template processing code in #46 that you might can re-use.

aem · 2020-04-23T13:54:47Z

@flybayer couple of thoughts re: your naming/architecture comments:

Rails and redwood use scaffold for generating everything

Agree that deviation is not ideal, although I do think the term scaffold is a bit wordy for what we're trying to convey. What if we just went with blitz generate all for that case? Feels like it might be a little more clear, accessible to anyone who doesn't natively speak English, and will save a few keystrokes :)

As for crud, it seems like that's just what I was using model for. Again, given that model feels like shared terminology I opted for that. I'm not super intent on that naming, crud just felt a little verbose/technical compared to model which felt a little more agnostic.

So I think using Context/User fits nicely for us. We should support both \ and /

Totally agree here. I hadn't gotten up to supporting nested paths yet, was building this pretty incrementally. Should be pretty simple to make this happen. I'll build in some fallback support for what to do if a particular context path doesn't exist (e.g. find best match, provide option to create the context resources similar to mkdir -p).

flybayer · 2020-04-23T14:05:30Z

Agree that deviation is not ideal, although I do think the term scaffold is a bit wordy for what we're trying to convey. What if we just went with blitz generate all for that case? Feels like it might be a little more clear, accessible to anyone who doesn't natively speak English, and will save a few keystrokes :)

Ok, great suggestion. Go with all for generating all :)

As for crud, it seems like that's just what I was using model for. Again, given that model feels like shared terminology I opted for that. I'm not super intent on that naming, crud just felt a little verbose/technical compared to model which felt a little more agnostic.

We definitely shouldn't use model for also generating queries/mutations. Model should be for an actual model, i.e. database table and model validation/authn.

I'm open to something better than crud for queries/mutations, but I think crud is very accurate here. Most people know what crud is. If not, they will learn quickly because it's a simple concept.

Totally agree here. I hadn't gotten up to supporting nested paths yet, was building this pretty incrementally. Should be pretty simple to make this happen. I'll build in some fallback support for what to do if a particular context path doesn't exist (e.g. find best match, provide option to create the context resources similar to mkdir -p).

Sounds good! Just make sure you finish the core stuff first before polishing. Alpha release will be about 21 hours from now.

flybayer · 2020-04-23T14:06:45Z

It's 9p for me now, so during my day tomorrow I can finish up some stuff if you need. Just let me know where you're at when you are done for your day :)

flybayer · 2020-04-24T03:42:30Z

Just curious why the template id directory is __id__ instead of [id]?

packages/cli/templates/page/__id__/edit.tsx

aem · 2020-04-24T04:28:33Z

@flybayer yeoman has a bug where it doesn't handle [] in filenames well, so in order to get the generator to recognize the files we have to use a different pattern and then replace it at runtime. I spent about half an hour and couldn't find a way around it so I decided this was the fastest way to an alpha

aem · 2020-04-24T04:33:17Z

Outstanding known issues that we'll want to fix (I can open issues for these):

When generating Prisma models the ConflictChecker always prompts to see if the user wants to commit the changes. We should build a way to disable the conflict checker in cases like this when we're manually writing to files but know that it's safe
The generated pages don't have any useful code in them, just a placeholder header telling the user what the page should be used for. We should generate some useful actions/components based on the prisma models (if they exist)
Never implemented component generator. I wasn't quite sure what we'd put in there yet, didn't seem worth the thought experiment at 12:30am ET 🙂
Context paths in the positional argument wasn't built out. I need to rewrite how we calculate the final object name + context path in order to make it a little more generic first, also seemed out of scope for the alpha so I opted to force users to provide context via the --parent flag for now. I'll start on the more flexible solution this weekend

All things considered, the generator is pretty robust right now. It's not incredibly smart as far as how it generates things, but definitely smart enough to give people something to play with this weekend. I'd be comfortable merging as is.

flybayer · 2020-04-24T04:46:21Z

Reorder the options like below (so help command prints in this order)

  All = 'all',
  Resource = 'resource',
  Crud = 'crud',
  Query = 'query',
  Mutation = 'mutation',
  Page = 'page',

Running outside a project logs error but then continues instead of quitting:

~/c/blitz> blitz g query task
✕ No blitz.config.js found. `generate` must be run from the root of the project.
? No parent flag (--parent, -p) was found. Would you like to create a new context folder under /app for 'tasks'? …
❯ Yes
  No

Change blitz g to print out the available generator commands and descriptions instead of this:

~/c/b/e/store> blitz g
 ›   Error: Missing 2 required args:
 ›   type  Type of files to generate
 ›   name  Namespace for generated files (e.g. [name]Query.ts).
 ›   See more help with --help

flybayer · 2020-04-24T04:48:43Z

High priority
Logged path of created files should have full path from root instead of this:

~/c/b/e/store> blitz g query task
✔ No parent flag (--parent, -p) was found. Would you like to create a new context folder under /app for 'tasks'? · Yes

CREATE    getTask.ts
CREATE    getTasks.ts

flybayer · 2020-04-24T08:28:26Z

@aem I disabled --context and the model generator for now. Let's open issues for them and make them nice without the pressure for this alpha :)

flybayer reviewed Apr 23, 2020

View reviewed changes

merelinguist suggested changes Apr 23, 2020

View reviewed changes

packages/cli/src/commands/generate.ts Outdated Show resolved Hide resolved

flybayer mentioned this pull request Apr 23, 2020

[legacy-framework] feat(cli): blitz generate command #46

Closed

flybayer closed this Apr 23, 2020

flybayer reopened this Apr 23, 2020

aem force-pushed the adam/generate-command-alpha branch from f59c6f7 to 4f3c27b Compare April 24, 2020 02:30

aem force-pushed the adam/generate-command-alpha branch from ac4ea88 to f7c32cc Compare April 24, 2020 04:27

aem marked this pull request as ready for review April 24, 2020 04:33

aem requested review from flybayer and merelinguist April 24, 2020 04:33

aem changed the title ~~initial scaffolding for generate command and validation in place~~ feat(cli): generate command Apr 24, 2020

aem added 6 commits April 24, 2020 01:52

initial scaffolding for generate command and validation in place

858d568

page generator

4a52cae

mutation generator

1c37110

add query and crud generator

a9e672e

move to server logger

b59153b

update docs with most recent API changes

9912788

aem added 3 commits April 24, 2020 01:53

undo logging changes in new command

8c6e811

rename a few things, clean up early returns

543d71c

use path context relative to the root

0647e12

aem force-pushed the adam/generate-command-alpha branch from 056688a to 0647e12 Compare April 24, 2020 05:55

aem and others added 4 commits April 24, 2020 02:13

Merge branch 'canary' into adam/generate-command-alpha

2087995

Merge branch 'canary' into adam/generate-command-alpha

9ee8fd8

lots of cleanup, changes, fixes

8c18c6e

fix test

3c1b17b

flybayer merged commit 1c7134c into canary Apr 24, 2020

flybayer deleted the adam/generate-command-alpha branch April 24, 2020 08:28

wKovacs64 mentioned this pull request Jul 7, 2022

Blitz repository inaccessible on Windows blitz-js/legacy-framework#831

Closed

dillondotzip changed the title ~~feat(cli): generate command~~ [legacy-framework] feat(cli): generate command Jul 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[legacy-framework] feat(cli): generate command #187

[legacy-framework] feat(cli): generate command #187

aem commented Apr 22, 2020 •

edited

Loading

flybayer Apr 23, 2020

aem Apr 23, 2020

flybayer Apr 23, 2020

flybayer Apr 23, 2020

aem Apr 23, 2020

flybayer commented Apr 23, 2020 •

edited

Loading

flybayer commented Apr 23, 2020 •

edited

Loading

flybayer commented Apr 23, 2020

flybayer commented Apr 23, 2020

aem commented Apr 23, 2020

flybayer commented Apr 23, 2020

flybayer commented Apr 23, 2020

flybayer commented Apr 24, 2020

aem commented Apr 24, 2020

aem commented Apr 24, 2020

flybayer commented Apr 24, 2020

flybayer commented Apr 24, 2020

flybayer commented Apr 24, 2020

[legacy-framework] feat(cli): generate command #187

[legacy-framework] feat(cli): generate command #187

Conversation

aem commented Apr 22, 2020 • edited Loading

Pull Request Type

Checklist

What's the reason for the change? ❓

What are the changes and their implications? ⚙️

Does this introduce a breaking change? ⚠️

Other information

flybayer Apr 23, 2020

Choose a reason for hiding this comment

aem Apr 23, 2020

Choose a reason for hiding this comment

flybayer Apr 23, 2020

Choose a reason for hiding this comment

flybayer Apr 23, 2020

Choose a reason for hiding this comment

aem Apr 23, 2020

Choose a reason for hiding this comment

flybayer commented Apr 23, 2020 • edited Loading

flybayer commented Apr 23, 2020 • edited Loading

flybayer commented Apr 23, 2020

flybayer commented Apr 23, 2020

aem commented Apr 23, 2020

flybayer commented Apr 23, 2020

flybayer commented Apr 23, 2020

flybayer commented Apr 24, 2020

aem commented Apr 24, 2020

aem commented Apr 24, 2020

flybayer commented Apr 24, 2020

flybayer commented Apr 24, 2020

flybayer commented Apr 24, 2020

aem commented Apr 22, 2020 •

edited

Loading

flybayer commented Apr 23, 2020 •

edited

Loading

flybayer commented Apr 23, 2020 •

edited

Loading