[cli] Introduce `--core` #3304

bharatkashyap · 2024-03-19T17:11:54Z

Running create-toolpad-app --core will bootstrap an empty Next.js application with the following file structure:

app/
  api/
    auth/
      [...nextauth]/
        route.ts
  auth/
    [...path]/
      page.tsx
  (dashboard)/
    page/
      page.tsx
    layout.tsx
  layout.tsx
theme.ts
.gitignore
tsconfig.json
.eslintrc.json
next.config.mjs
package.json

Placeholder start page:
Screen.Recording.2024-03-19.at.10.48.56.PM.mov

apedroferreira

Looks good!
I left some comments but nothing necessarily blocking I think.

apedroferreira · 2024-03-20T11:48:13Z

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

+  // eslint-disable-next-line no-console
+  console.log();
+
+  const packageJson: PackageJson = {


Could we have the generated template as a folder instead of all in the file? Probably would be easier to work on.
For example Next.js and Vite have it like that: https://github.com/vercel/next.js/tree/canary/packages/create-next-app/templates https://github.com/vitejs/vite/tree/main/packages/create-vite

Jan and I did discuss this and felt like we could eventually move to this structure, but begin with strings for simplicity and efficiency

If the idea is that in the future we will do things like pre-configure different auth providers, we're going to need to have some templating mechanism. e.g. to have different modules in the package json, or to generate the code that initializes the providers.
I believe this mechanism will be much easier to write and maintain when it's just composition in functions, rather than try to adjust pre-existing file content from a template file.

It'll also make it easier to do things like re-using the templating logic to generate codesandboxes in the browser.

apedroferreira · 2024-03-20T11:51:12Z

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

+      return (
+        <html lang="en">
+          <body>
+            <AppRouterCacheProvider>


Here will just need to use the AppProvider from @toolpad/core:

<html lang="en"> <body> <AppProvider theme={theme}>{props.children}</AppProvider> </body> </html>

It should be ready in #3291 I'm just sorting out some CI issues.
I can replace it if I merge after.

apedroferreira · 2024-03-20T11:52:04Z

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

+  await execaCommand(command, { stdio: 'inherit', cwd: absolutePath });
+
+  // Create the `app` directory
+  await fs.mkdir(path.join(absolutePath, 'app'));


Maybe we could use Promise.all to make it faster? Probably not a huge difference though.

apedroferreira · 2024-03-20T11:52:48Z

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

+    display: "swap",
+  });
+
+  const theme = createTheme({


All looks good, I was using the exact same project setup when setting up @toolpad/core!

apedroferreira · 2024-03-20T12:00:42Z

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

+  `;
+
+  // Create the `theme.ts` file in the root directory
+  await fs.writeFile(path.join(absolutePath, 'theme.ts'), themeContent);


So we're not using an src folder? I prefer to use one just to organize the files a bit more but any option is fine, we just never discussed it.

Janpot · 2024-03-21T09:12:12Z

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

@@ -36,7 +36,7 @@ function getPackageManager(): PackageManager {
      return 'npm';
    }
  }
-  return 'yarn';
+  return 'pnpm';


The default should probably be npm as that is bundled with each node installation (at least for now)

Janpot · 2024-03-21T09:22:09Z

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

+
+  const installVerb = 'install';
+  const command = `${packageManager} ${installVerb}`;
+  await execaCommand(command, { stdio: 'inherit', cwd: absolutePath });


If possible, I'd use the safer

await execa(packageManager, ['install'], { stdio: 'inherit', cwd: absolutePath });

command. It'll correctly escape arguments. See https://www.npmjs.com/package/execa#execafile-arguments-options

https://www.npmjs.com/package/execa#execacommandcommand-options also provides automatic escaping of arguments - is execa safer in any other way?

it escapes spaces as well

…o cta-core-1

Janpot · 2024-03-21T15:05:58Z

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

+        fs.mkdir(path.join(absolutePath, dirPath), { recursive: true }),
+      ),
+    );
+  } catch (error: any) {


You can remove this try/catch. mkdir doesn't throw EEXIST when recursive is true

Suggested change

} catch (error: any) {

Also the try/catch is around the Promise.all meaning it triggers immediately on the first error. That doesn't mean the other items are finished creating. This sort of logic would've belonged inside of the loop.

Janpot

Looks good!

Suggestion: to clean it up a bit I'd consider extracting the logic in separate functions.

function generateProject(options): Map<string, { content: string }>;
async function writeFiles(path, files: Map<string, { content: string }>): void;

And place each in their own module.

…o cta-core-1

apedroferreira

Looks great!

Janpot · 2024-03-22T17:50:26Z

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

+import path from 'path';
+import fs from 'fs/promises';
+
+export default async function writeCoreFiles(


Is this function really specific to core? Seems quite general to me

Suggested change

export default async function writeCoreFiles(

export default async function writeFiles(

bharatkashyap added 3 commits March 19, 2024 18:05

feat: Add @toolpad/core bootstrap under --core

72cfab1

fix: Types and /redirect

f03ebb8

fix: Add to description

7c11264

bharatkashyap added the create-toolpad-app Issues related to the `create-toolpad-app` CLI tool label Mar 19, 2024

Merge branch 'master' into cta-core-1

7f231fa

apedroferreira approved these changes Mar 20, 2024

View reviewed changes

bharatkashyap requested a review from Janpot March 21, 2024 06:49

Janpot reviewed Mar 21, 2024

View reviewed changes

bharatkashyap added 6 commits March 21, 2024 17:03

fix: Review 1

fdecc4c

Merge branch 'master' into cta-core-1

96d62fc

fix: Keep for legacy reasons

da912fd

feat: Use @toolpad/core

c7dd5cd

Merge branch 'cta-core-1' of github.com:bharatkashyap/mui-toolpad int…

fe5f187

…o cta-core-1

Merge branch 'master' into cta-core-1

ea01ba9

bharatkashyap requested a review from Janpot March 21, 2024 13:42

Janpot reviewed Mar 21, 2024

View reviewed changes

fix: No EEXIST when recursive: true

6087545

bharatkashyap requested review from Janpot and apedroferreira March 22, 2024 10:36

Janpot approved these changes Mar 22, 2024

View reviewed changes

bharatkashyap added 4 commits March 22, 2024 18:35

fix: Encapsulate better, Jan review

0391f0f

fix: Clean up

0acec47

Merge branch 'master' into cta-core-1

facd169

Merge branch 'cta-core-1' of github.com:bharatkashyap/mui-toolpad int…

3023651

…o cta-core-1

apedroferreira approved these changes Mar 22, 2024

View reviewed changes

Janpot reviewed Mar 22, 2024

View reviewed changes

bharatkashyap added 2 commits March 26, 2024 14:43

feat: Basic functional to starter nav

ad46961

fix: Jan review

ba54a15

bharatkashyap merged commit c93afe3 into mui:master Mar 26, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] Introduce `--core` #3304

[cli] Introduce `--core` #3304

bharatkashyap commented Mar 19, 2024 •

edited

Loading

apedroferreira left a comment

apedroferreira Mar 20, 2024

bharatkashyap Mar 20, 2024

Janpot Mar 21, 2024 •

edited

Loading

apedroferreira Mar 20, 2024

apedroferreira Mar 20, 2024

apedroferreira Mar 20, 2024

apedroferreira Mar 20, 2024 •

edited

Loading

Janpot Mar 21, 2024

Janpot Mar 21, 2024 •

edited

Loading

bharatkashyap Mar 21, 2024

Janpot Mar 21, 2024

Janpot Mar 21, 2024 •

edited

Loading

Janpot left a comment •

edited

Loading

apedroferreira left a comment

Janpot Mar 22, 2024

	export default async function writeCoreFiles(
	export default async function writeFiles(

[cli] Introduce --core #3304

[cli] Introduce --core #3304

Conversation

bharatkashyap commented Mar 19, 2024 • edited Loading

apedroferreira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot Mar 21, 2024 • 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

apedroferreira Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Janpot left a comment • edited Loading

Choose a reason for hiding this comment

apedroferreira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[cli] Introduce `--core` #3304

[cli] Introduce `--core` #3304

bharatkashyap commented Mar 19, 2024 •

edited

Loading

Janpot Mar 21, 2024 •

edited

Loading

apedroferreira Mar 20, 2024 •

edited

Loading

Janpot Mar 21, 2024 •

edited

Loading

Janpot Mar 21, 2024 •

edited

Loading

Janpot left a comment •

edited

Loading