feat(create-waku): install dependencies automatically when creating a new waku #728

Rec0iL99 · 2024-05-28T16:57:10Z

Workflow:

> pnpm start                                                                                                                        

> create-waku@0.9.2-alpha.0 start /Users/joelmathew/WebProjects/waku-dev/waku/packages/create-waku
> node ./dist/index.js

✔ Project Name … waku-project
Setting up project...

Installing dependencies by running pnpm install...
Scope: all 32 workspace projects
Done in 1.6s

Done. Now run:

cd waku-project
pnpm dev

vercel · 2024-05-28T16:57:14Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated (UTC)
waku	⬜️ Ignored (Inspect)	Visit Preview	Jun 26, 2024 3:13pm

codesandbox-ci · 2024-05-28T16:57:33Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

dai-shi

Hi, thanks for working on it! Please see comments.

cc @ojj1123 would you like to give a review?

packages/create-waku/package.json

packages/create-waku/src/index.ts

ojj1123

Good works! I review for something.

packages/create-waku/src/index.ts

dai-shi · 2024-06-20T15:34:44Z

@Rec0iL99 Are you still around?

Rec0iL99 · 2024-06-20T15:45:43Z

@Rec0iL99 Are you still around?

@dai-shi sorry to keep this open for a while now. I had intended to finish this last weekend but got caught up with some work. I'll finish this by the weekend.

packages/create-waku/src/index.ts

himself65 · 2024-06-25T18:18:50Z

packages/create-waku/src/index.ts

+
+  process.chdir(targetDir);
+
+  const installProcess = spawn(packageManager, ['install']);


Suggested change

const installProcess = spawn(packageManager, ['install']);

const installProcess = spawn(packageManager, ['install'], { encoding: 'utf8' });

Hi @himself65, encoding is not a property that exists on the options object for spawn.

first to know this

Oh, I think if you just wanna bypass the output, use stdio: 'inherit'

dai-shi · 2024-06-26T02:20:30Z

packages/create-waku/src/index.ts

+
+  installProcess.stdout.setEncoding('utf8');
+  installProcess.stdout.on('data', (data) => {
+    console.log(data);


#728 (comment)

Does it show the indicator as expected? cc @ojj1123

Yes. I could see the indicator as expected!

#728 (comment) I wonder if stdio: inherit works.

The docs is explaining a difference between stdio: "inherit" and stdio: "pipe" but it's a bit confusing for me.
I've tried to use stdio: "inherit", it could also show the loading indicator.

stdio: "pipe"(default)

child stdios(stdin, stdout, stderr) and parent ones are mapped(piping) each other. But if we want to access the data, we have to use the events like this:

childProcess.stdout.on("data", (data) => { console.log(data) })

stdio: "inherit"

parent's stdios are handed off to the child process. That is, child process could use parent's stdios. So we don't need to use the events.

I think no matter we use stdio: "pipe" or stdio: "inherit". But if we use "inherit", we don't need to use the events:

diff --git a/packages/create-waku/src/index.ts b/packages/create-waku/src/index.ts index 1eb01d3..98ede50 100644 --- a/packages/create-waku/src/index.ts +++ b/packages/create-waku/src/index.ts @@ -210,16 +210,8 @@ async function init() { process.chdir(targetDir); - const installProcess = spawn(packageManager, ['install']); - - installProcess.stdout.setEncoding('utf8'); - installProcess.stdout.on('data', (data) => { - console.log(data); - }); - - installProcess.stderr.setEncoding('utf8'); - installProcess.stderr.on('data', (data) => { - console.log(data); + const installProcess = spawn(packageManager, ['install'], { + stdio: 'inherit', }); installProcess.on('close', (code) => {

+) If we use stdio: "inherit"(or anything other than "pipe"), childProcess's stdin, stdout and stderr could be null:

const installProcess = spawn(packageManager, ['install'], { stdio: 'inherit', }); installProcess.stdout // it could be null so we need to null-check installProcess.stdout.on("data", callbackFn)

himself65 · 2024-06-27T00:21:14Z

packages/create-waku/src/index.ts

+
+  process.chdir(targetDir);
+
+  const installProcess = spawn(packageManager, ['install']);


Oh, I think if you just wanna bypass the output, use stdio: 'inherit'

dai-shi · 2024-07-23T12:47:49Z

@Rec0iL99 Are you still around?

ojj1123 · 2024-07-25T04:52:46Z

If @Rec0iL99 is busy and there is no response, it's okay for me to get this work.

dai-shi · 2024-07-25T05:13:56Z

@ojj1123 Yeah, please go ahead.

ojj1123 · 2024-07-28T14:45:56Z

Yes. I could see the indicator as expected!
#728 (comment)

Oh it does NOT show the loading indicator on installing. I got mistakes. 😅
We should use stdio: "inherit"!

I couldn't push my commit to this PR, so I made a new PR 👉 #808

Rec0iL99 added 2 commits May 28, 2024 11:48

feat: install dependencies automattically when creating the waku project

b915f15

dynamic package manager

55e5206

dai-shi reviewed May 29, 2024

View reviewed changes

packages/create-waku/package.json Outdated Show resolved Hide resolved

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

ojj1123 reviewed May 30, 2024

View reviewed changes

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

dai-shi requested changes May 30, 2024

View reviewed changes

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

himself65 suggested changes Jun 21, 2024

View reviewed changes

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

removed cross-spawn and used spawn from node:child_process

26eb8ba

Rec0iL99 requested review from ojj1123, himself65 and dai-shi June 25, 2024 18:17

himself65 suggested changes Jun 25, 2024

View reviewed changes

packages/create-waku/src/index.ts Outdated Show resolved Hide resolved

himself65 suggested changes Jun 25, 2024

View reviewed changes

dai-shi reviewed Jun 26, 2024

View reviewed changes

process is a global env

6e1dddf

himself65 suggested changes Jun 27, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

ojj1123 mentioned this pull request Jul 28, 2024

feat(create-waku): install dependencies automatically when creating a new waku project #808

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(create-waku): install dependencies automatically when creating a new waku #728

feat(create-waku): install dependencies automatically when creating a new waku #728

Rec0iL99 commented May 28, 2024 •

edited

Loading

vercel bot commented May 28, 2024 •

edited

Loading

codesandbox-ci bot commented May 28, 2024 •

edited

Loading

dai-shi left a comment

ojj1123 left a comment

dai-shi commented Jun 20, 2024

Rec0iL99 commented Jun 20, 2024

himself65 Jun 25, 2024

Rec0iL99 Jun 26, 2024 •

edited

Loading

himself65 Jun 26, 2024

himself65 Jun 27, 2024

dai-shi Jun 26, 2024

ojj1123 Jun 27, 2024

dai-shi Jun 27, 2024

ojj1123 Jul 1, 2024 •

edited

Loading

ojj1123 Jul 1, 2024 •

edited

Loading

himself65 Jun 27, 2024

dai-shi commented Jul 23, 2024

ojj1123 commented Jul 25, 2024 •

edited

Loading

dai-shi commented Jul 25, 2024

This comment was marked as outdated.

ojj1123 commented Jul 28, 2024 •

edited

Loading


		process.chdir(targetDir);

		const installProcess = spawn(packageManager, ['install']);

feat(create-waku): install dependencies automatically when creating a new waku #728

Are you sure you want to change the base?

feat(create-waku): install dependencies automatically when creating a new waku #728

Conversation

Rec0iL99 commented May 28, 2024 • edited Loading

vercel bot commented May 28, 2024 • edited Loading

codesandbox-ci bot commented May 28, 2024 • edited Loading

dai-shi left a comment

Choose a reason for hiding this comment

ojj1123 left a comment

Choose a reason for hiding this comment

dai-shi commented Jun 20, 2024

Rec0iL99 commented Jun 20, 2024

himself65 Jun 25, 2024

Choose a reason for hiding this comment

Rec0iL99 Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

himself65 Jun 26, 2024

Choose a reason for hiding this comment

himself65 Jun 27, 2024

Choose a reason for hiding this comment

dai-shi Jun 26, 2024

Choose a reason for hiding this comment

ojj1123 Jun 27, 2024

Choose a reason for hiding this comment

dai-shi Jun 27, 2024

Choose a reason for hiding this comment

ojj1123 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

stdio: "pipe"(default)

stdio: "inherit"

ojj1123 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

himself65 Jun 27, 2024

Choose a reason for hiding this comment

dai-shi commented Jul 23, 2024

ojj1123 commented Jul 25, 2024 • edited Loading

dai-shi commented Jul 25, 2024

This comment was marked as outdated.

ojj1123 commented Jul 28, 2024 • edited Loading

Rec0iL99 commented May 28, 2024 •

edited

Loading

vercel bot commented May 28, 2024 •

edited

Loading

codesandbox-ci bot commented May 28, 2024 •

edited

Loading

Rec0iL99 Jun 26, 2024 •

edited

Loading

ojj1123 Jul 1, 2024 •

edited

Loading

`stdio: "pipe"`(default)

`stdio: "inherit"`

ojj1123 Jul 1, 2024 •

edited

Loading

ojj1123 commented Jul 25, 2024 •

edited

Loading

ojj1123 commented Jul 28, 2024 •

edited

Loading