better CLI #170
better CLI #170
Conversation
src/utils.ts
Outdated
| @@ -0,0 +1,10 @@ | |||
| import * as fs from 'fs'; | |||
|
|
|||
| export function exists(file: string) { | |||
There was a problem hiding this comment.
huh, I thought it was deprecated. Turns out that's just fs.exists. Nice!
|
|
||
| const export_dir = opts._[1] || 'export'; | ||
| const start = Date.now(); |
There was a problem hiding this comment.
You can use process.hrtime if you want to be a little more exact. Roughly the same though
There was a problem hiding this comment.
when pretty-ms is involved I don't think it'll make any difference. I generally avoid process.hrtime because, well, the signature is batshit
| process.env.NODE_ENV = 'production'; | ||
| process.env.SAPPER_DEST = '.sapper/.export'; |
There was a problem hiding this comment.
More for my own curiosity, but why not set SAPPER_DEST from within export()? Especially since the dest is variable
There was a problem hiding this comment.
It's used by build first — it sends the webpack output to .sapper/.export, and then sapper export runs the code therein to generate the export directory. So it has to be set before either thing happens
src/cli/index.ts
Outdated
| .replace(/^(.+)/gm, (m: string, $1: string) => /[#>]/.test(m) ? $1 : ` ${$1}`) | ||
| .replace(/^# (.+)/gm, (m: string, $1: string) => chalk.bold.underline($1)) | ||
| .replace(/^> (.+)/gm, (m: string, $1: string) => chalk.cyan($1)); | ||
| prog.version(pkg.version); |
There was a problem hiding this comment.
Definitely personal preference, but I like to recommend assigning all global aspects into the initial assignment. New commands get their own blocks, and that consistent spacing helps separate things with larger CLIs.
const prog = sade('sapper').version(pkg.version);`There was a problem hiding this comment.
Ah, nice. I didn't realise it was chainable, I suppose that makes sense!
There was a problem hiding this comment.
Aye. You can chain the whole thing if you want, but that'll get messy
|
This PR ended up including a bunch of changes to the tests. They weren't very well structured before, and it turned out they had certain assumptions (about the build directory etc) that weren't necessarily true — in fact, the Getting dev tests working will require jumping through a few more hoops, so I've just commented them out for now. Everything else is a bit neater and more robust. |
Fixes #168 and #169, using another @lukeed jam (Sade) which gives us a much nicer CLI.
sapper dev,sapper startandsapper exportwill all now pick available ports as necessary, unless (fordevandstart) you specify a--portor-p, in which case it will try to use that port and print a useful error if it can't.It also emits stats following the build, and prints instructions on how to start the app.