Add create-toolpad-app CLI #1700

bharatkashyap · 2023-02-22T13:54:27Z

Part of Getting started with Toolpad to be improved #1653
Visual demonstration

CTA.mov

Janpot · 2023-03-16T13:40:01Z

packages/create-toolpad-app/index.ts

+  console.log(`Creating a new MUI Toolpad project in ${chalk.blue(name.projectName)}`);
+  try {
+    await fs.mkdir(name.projectName);
+    process.chdir(name.projectName);


Suggested change

process.chdir(name.projectName);

Can this be avoided? e.g. by passing cwd to execa instead. I feel like this could lead to weird issues where half the program is working in a certain working dir, and the other half in another. Moving code around could become awkward

Janpot · 2023-03-16T13:43:21Z

packages/create-toolpad-app/index.ts

+};
+
+// Create a new directory and initialize a new project
+const scaffoldProject = async () => {


This is a nit, but nothing about the name scaffoldProject suggests it would be returning a string or what that string means. It feels like

Suggested change

const scaffoldProject = async () => {

const scaffoldProject = async (name: string): Promise<void> => {

and calling inquirer before calling this function would make more sense to me.

Janpot · 2023-03-16T13:45:58Z

packages/create-toolpad-app/index.ts

+    return name.projectName;
+  } catch (error) {
+    // Directory exists, verify if it is empty to continue
+    if (!isFolderEmpty(path.join(process.cwd(), name.projectName), name.projectName)) {


Ideally, you read process.cwd() once and pass it around. Especially when you'll be doing process.chdir in some places.
The purer a functions is, the easier to debug and test.

Janpot · 2023-03-16T13:46:33Z

packages/create-toolpad-app/utils/isFolderEmpty.ts

+    console.log();
+    for (const file of conflicts) {
+      try {
+        const stats = fs.lstatSync(path.join(root, file));


Avoid sync methods

Janpot · 2023-03-16T13:46:47Z

packages/create-toolpad-app/utils/isFolderEmpty.ts

+  ];
+
+  const conflicts = fs
+    .readdirSync(root)


Avoid sync methods

Janpot · 2023-03-16T13:48:35Z

packages/create-toolpad-app/utils/isFolderEmpty.ts

+import path from 'path';
+
+export function isFolderEmpty(root: string, name: string): boolean {
+  const validFiles = [


Are we really going to curate a list with random files? Why not just be strict? empty == no files?

Fair enough, I imagined that we could rely on the list curated by https://github.com/vercel/next.js/blob/canary/packages/create-next-app/helpers/is-folder-empty.ts but it might make sense to keep it simple and follow the strict definition of an empty folder to begin with

Ah, that could make sense then

Should we leave it in, then?

As you wish

Janpot · 2023-03-16T13:51:56Z

packages/create-toolpad-app/utils/isFolderEmpty.ts

+
+  if (conflicts.length > 0) {
+    // eslint-disable-next-line no-console
+    console.log(`The directory ${chalk.green(name)} contains files that could conflict:`);


This seems to present itself as a reusable utility function isFolderEmpty, but then proceeds to do very specific logic for certain files, adds logging, etc... This function is not a reusable utility. I'd remove this file and colocate the code where you are using it.

Janpot · 2023-03-16T13:53:48Z

packages/create-toolpad-app/index.ts

For package structure I'd propose to use a src folder, just like the other projects. Also not sure whether there is real benefit to add a utils folder in a project that has 3 source files

Janpot · 2023-03-17T13:55:46Z

packages/create-toolpad-app/package.json

+  "author": "Toolpad Team",
+  "license": "MIT",
+  "bin": {
+    "create-toolpad-app-test": "./dist/index.js"


Suggested change

"create-toolpad-app-test": "./dist/index.js"

"create-toolpad-app": "./dist/index.js"

Also, does this file have execute permissions? I didn't think tsc dealt with that. In the other package we use a top level javascript file with execute permissions that imports from the dist folder

Also, does this file have execute permissions?

What do you mean by this exactly/How do I test this?

which command do you use to test the cli at the moment?

In development, I run the built file from the dist folder in a separate folder through node; to test the production version, I publish the package and run create toolpad-app to test the flow

Oh wow, it looks like npm automatically sets the correct file permissions when you use the bin field. I didn't know that. Maybe we should do the same for the @mui/toolpad cli later on.

Janpot · 2023-03-17T13:57:09Z

packages/create-toolpad-app/package.json

+  "files": [
+    "dist"
+  ],
+  "type": "module",


Suggested change

"type": "module",

?

Without this I get

SyntaxError: Cannot use import statement outside a module

errors when running the compiled .js file,

Or alternatively, if compiling to es5, a

Error [ERR_REQUIRE_ESM]: require() of ES Module MUI/mui-studio/packages/create-toolpad-app/node_modules/chalk/source/index.js from MUI/mui-studio/packages/create-toolpad-app/dist/index.js not supported.

Fixed this by dynamically importing all the ES modules (chalk, inquirer and execa)

Janpot · 2023-03-17T13:57:45Z

packages/create-toolpad-app/package.json

+  ],
+  "type": "module",
+  "scripts": {
+    "dev13": "tsc -w -p ./tsconfig.json --outDir ./dist/",


Can outDir go in tsconfig.json?

Janpot · 2023-03-17T13:59:16Z

packages/create-toolpad-app/package.json

+  "type": "module",
+  "scripts": {
+    "dev13": "tsc -w -p ./tsconfig.json --outDir ./dist/",
+    "prerelease": "rimraf ./dist/",


"prebuild": "rimraf ./dist/", "build": "tsc -p ./tsconfig.json -outDir ./dist/",

Let's use the same script names as in @mui/toolpad-core and @mui/toolpad-components?

Janpot · 2023-03-17T13:59:49Z

packages/create-toolpad-app/package.json

+  "dependencies": {},
+  "devDependencies": {
+    "@types/inquirer": "^9.0.3",
+    "chalk": "^5.2.0",


I think these will need to go in dependencies when we don't use ncc

does @types/inquirer have to go in dependencies as well?

Nope, devDependencies should be fine

Janpot · 2023-03-17T14:00:20Z

packages/create-toolpad-app/src/index.ts

+  console.log(`Creating a new MUI Toolpad project in ${chalk.blue(projectName)}`);
+  try {
+    await fs.mkdir(projectName);
+    // process.chdir(projectName);


Suggested change

// process.chdir(projectName);

Janpot · 2023-03-17T14:01:54Z

packages/create-toolpad-app/src/index.ts

+
+  if (files.length > 0) {
+    // eslint-disable-next-line no-console
+    console.log(`The directory ${chalk.green(name)} is non-empty.`);


Nit: does it make sense to move this logging to the consumer of isFolderEmpty? The function name doesn't suggest it will have side-effects.

Janpot · 2023-03-19T08:55:09Z

Great work!

Janpot added 30 commits February 1, 2023 16:11

dsafa

c81485d

alpha 3

58b3edd

number formatter

398eb18

fix preview

93511b6

wip

e6b5c23

add note

9fb3450

Merge branch 'master' into pubs

64883a2

Update NOTES.md

e5573bd

Merge branch 'master' into pubs

e5ffa2a

Forbid writing in start mode

94f3261

forbid creating new app

dcde747

dsefrw

24aadb1

upgrade

9a291cc

Merge branch 'master' into pubs

a79d329

Add path

c5a445c

dewd

3e6a65a

Merge branch 'master' into pubs

4ec5d26

env

175fede

dewfrewr

92ef860

fix empty components folder

bbd7182

fdsfds

2cd5cc0

maybe env

e850d20

dfewfw

386d727

keep env

59afbe2

dffew

490f6bd

Merge branch 'master' into pubs

e241ccd

dwefewr

5807af4

local workflow

68f0366

Update index.tsx

7c96e16

componnet by name

bec7d54

Janpot reviewed Mar 16, 2023

View reviewed changes

bharatkashyap added 2 commits March 17, 2023 09:11

Fix: Review

d88f45b

Fix: Match version with the rest of the packages

5188b4e

Janpot reviewed Mar 17, 2023

View reviewed changes

bharatkashyap added 2 commits March 18, 2023 12:13

Fix: Review and module resolution fixes

9acb3fd

Fix: Remove types from deps

f4927da

Janpot approved these changes Mar 19, 2023

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 19, 2023

Base automatically changed from direction-13 to master March 20, 2023 08:03

Janpot added 2 commits March 20, 2023 09:06

Merge branch 'master' into create-toolpad-app

6adb57d

Merge branch 'master' into create-toolpad-app

8bbef1b

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 20, 2023

remove

d4b4ccf

Janpot enabled auto-merge (squash) March 20, 2023 08:35

Janpot merged commit efd6638 into master Mar 20, 2023

Janpot deleted the create-toolpad-app branch March 20, 2023 08:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add create-toolpad-app CLI #1700

Add create-toolpad-app CLI #1700

bharatkashyap commented Feb 22, 2023 •

edited

Loading

Janpot Mar 16, 2023

Janpot Mar 16, 2023

Janpot Mar 16, 2023 •

edited

Loading

Janpot Mar 16, 2023

Janpot Mar 16, 2023

Janpot Mar 16, 2023

bharatkashyap Mar 16, 2023 •

edited

Loading

Janpot Mar 17, 2023

bharatkashyap Mar 17, 2023

Janpot Mar 17, 2023

Janpot Mar 16, 2023

Janpot Mar 16, 2023

Janpot Mar 17, 2023 •

edited

Loading

bharatkashyap Mar 17, 2023

Janpot Mar 18, 2023

bharatkashyap Mar 18, 2023 •

edited

Loading

Janpot Mar 19, 2023

Janpot Mar 17, 2023

bharatkashyap Mar 17, 2023 •

edited

Loading

bharatkashyap Mar 18, 2023

Janpot Mar 17, 2023

Janpot Mar 17, 2023

Janpot Mar 17, 2023

Janpot Mar 18, 2023 •

edited

Loading

bharatkashyap Mar 18, 2023

Janpot Mar 17, 2023

Janpot Mar 17, 2023

Janpot commented Mar 19, 2023

	const scaffoldProject = async () => {
	const scaffoldProject = async (name: string): Promise<void> => {

	"create-toolpad-app-test": "./dist/index.js"
	"create-toolpad-app": "./dist/index.js"

Add create-toolpad-app CLI #1700

Add create-toolpad-app CLI #1700

Conversation

bharatkashyap commented Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot Mar 16, 2023 • 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

bharatkashyap Mar 16, 2023 • 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

Choose a reason for hiding this comment

Janpot Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bharatkashyap Mar 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bharatkashyap Mar 17, 2023 • 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

Janpot Mar 18, 2023 • 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

Janpot commented Mar 19, 2023

bharatkashyap commented Feb 22, 2023 •

edited

Loading

Janpot Mar 16, 2023 •

edited

Loading

bharatkashyap Mar 16, 2023 •

edited

Loading

Janpot Mar 17, 2023 •

edited

Loading

bharatkashyap Mar 18, 2023 •

edited

Loading

bharatkashyap Mar 17, 2023 •

edited

Loading

Janpot Mar 18, 2023 •

edited

Loading