[legacy-framework] feat(cli): Add initial CLI

[flybayer]

This replaces #7 by @mabadir. This branch is native to the blitz repo so we can all commit easily.

[dkempner]

lgtm

[medelman17]

Seems to be working within the monorepo just fine. Tests failing do not appear to be monorepo-related. Thus, I would say that once the above review is resolved and tests are passing, it's ready to merge.

[flybayer]

Current pending items before we can merge this initial work:

• Switch testing to jest
• Get tests passing
• Fix yarn build — currently failing

[lorenzorapetti]

Some thoughts:

blitz new [PATH]

Right now this command takes PATH as an argument but the generator uses the same argument to get the app name. Should we separate path and app name so that the user can write blitz new ./projects and then the system asks for the name? We should standardize this so we can use the same standard for all the generators.

Commands

Except for the new command, all the others don't do anything, they're just stubs. I think we should either remove them or implement them before we merge.

Abstract the generator code

Quite a bit of the generator code can be abstracted for later reuse by other generators, should we do it now or later?

[flybayer]

Awesome @lorenzorapetti!! For now we can just use the directory name. So if path is /tmp/blitz the name is blitz. If folks want it, we can add a prompt and flag to specify the name.

Also, as a guiding principle, word the CLI prompts such that Yes is always the default.

[attaradev]

I think we should use blitz new [NAME] so we make the path relative to the current directory, just as in CRA

[flybayer]

@mikeattara blitz new [PATH] does what CRA does but also accepts a directory in another location.

blitz new app -> ./app/ with name app
blitz new projects/app -> ./projects/app/ with name app
blitz new ../app -> ../app/ with name app

[lorenzorapetti]

Ok, now the tests are passing! Too bad that we don't have any yet 😅. I need to study how to mock inquirer.js.

Should we have something in place to define where to put test files (e.g. local .test.ts file vs. global __tests__ or test directory)?

[mabadir]

We might also want to have global jest config.

[jasonblalock]

Just curious since we're changing the repo formatting as part of this CLI PR, is this the standard that we want to adopt for all blitzjs repos (website repo included)?

[flybayer]

I added this. I'm willing to flex on the printWidth, but not the semi 😄

[jasonblalock]

Oh man, that's it I'm out:door: J/k, I can ignore the lack of semi :laughing: And I don't have an opinion on the print width, I've always gone with the default based on their recommendation https://prettier.io/docs/en/options.html. Mostly was just curious so we can keep the website repo in sync.

[jasonblalock]

This is probably my being new to Typescript, but do we not need to target es5? Or are we assuming users of our library will transpile our es6/es2015 as part of their build?

And same question for the es2017 in packages/cli/tsconfig.json

[flybayer]

Yes, we need to leave this as es5 like @medelman17 set up. If the CLI has different needs, then overside the top level tsconfig in the CLI tsconfig.

[lorenzorapetti]

I reverted the change. I'm just curious, why do we need to compile down to es5? The latest node LTS supports es2019

[flybayer]

@lorenzorapetti es5 is for the other packages, not the cli. The cli can target something newer.

[flybayer]

I really don't like __tests__ :D So use a combination of tests directory and a.test.ts depending on what makes the most sense to y'all.

[jasonblalock]

I've been a fan of the local tests in the past. Feels like it removes the friction of clicking through directories to get to the one that matches your src. It's just the file right below the one you're working on. Just my two cents.

[medelman17]

Tests should all go in one place. And it should be the same place in each package in the monorepo. Consistency across the monorepo is one of its KEY benefits. To do otherwise would be to foist ourselves upon our own petards. Cheers! Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Jason Blalock <notifications@github.com> Sent: Tuesday, February 25, 2020 10:20:25 PM To: blitz-js/blitz <blitz@noreply.github.com> Cc: Michael Edelman <michael@fabulas.io>; Mention <mention@noreply.github.com> Subject: Re: [blitz-js/blitz] Add initial CLI (#8) I've been a fan of the local tests in the past. Feels like it removes the friction of clicking through directories to get to the one that matches your src. It's just the file right below the one you're working on. Just my two cents. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#8?email_source=notifications&email_token=ADQ3VLPNCUMUVTRRIRVR4Q3REXNXTA5CNFSM4KZ5FAV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM6UL4Y#issuecomment-591218163>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADQ3VLJMYKOJCIUVFBPCD3TREXNXTANCNFSM4KZ5FAVQ>.

[kamerontanseli]

Problem

Tried running the cli new within an empty directory to check if everything generates okay.

kamerontrayio:example kamerontanseli$ ../packages/cli/bin/run new .
? How do you want your project to be called? example
   create package.json
   create README.md
   create pages/index.js
   create .gitignore
yarn add v1.22.0
error package.json: Name contains illegal characters
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.
App created!

NPM complains about the package.json name. It should be just example. Note: for some reason an invalid package name stops anything from being added as dependencies.

{
  "name": "/Users/kamerontanseli/Desktop/blitz/example",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start"
  }
}

Solution

If we use path.basename in _packageJson then the end result will give us the folder name and install the deps properly:

_packageJson(name: string) {
    return {
      name: path.basename(name),
      version: '0.0.1',
      private: true,
      scripts: {
        dev: 'next dev',
        build: 'next build',
        start: 'next start',
      },
    }
  }

{
  "name": "example",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start"
  },
  "dependencies": {
    "next": "^9.2.2",
    "react": "^16.12.0",
    "react-dom": "^16.12.0"
  }
}

[lorenzorapetti]

Thank you for reporting! I've just pushed a fix

[flybayer]

Fyi: I'm working right now on merging this into canary