Developer-DAO / create-dao

MIT License
93 stars 15 forks source link

first pass at wrapped create-dao cli - Resolves #6 #9

Closed henripal closed 2 years ago

henripal commented 2 years ago

This commit sets up the CLI with commander, following the create-next-app architecture.

what does this do

The user, after installing create-dao can now run npx create-dao <daoname> and a daoname directory will be created with the contents of the template directory.

If no directory is specified, the user will be prompted for a project name.

Similar to create-next-app, there's a lot of room here to create several templates in the template directory and guide the user to the right template with prompts.

how this was done:

mostly copying the code from create-next-app and modifying it for our use case. This is maybe overkill for now, but assuming we have several templates, we want to give the user option to use typescript, or not, etc... the create-next-app scaffold will be immediately useable.

Dhaiwat10 commented 2 years ago

@henripal awesome PR, thanks!

I keep seeing these when I'm trying to run yarn tho

image

henripal commented 2 years ago

seems the same as https://github.com/vercel/ncc/issues/622 ? can't reproduce locally, but I added the fix proposed in the issue above in commit 943b0f5 (adding TS in devDependencies)

Dhaiwat10 commented 2 years ago

Let's also move dist to gitignore @henripal

Forgot to add it to my review.

with-heart commented 2 years ago

Working on a review throughout today, gimme a bit :D

henripal commented 2 years ago

Thanks for all the comments - pushed fixes.

NB: I left the example boilerplate for a future version where the template folder would be replaced by templates containing several different DAO templates: templates/basic, templates/nftonly, etc...

with-heart commented 2 years ago

I think I agree with that you that this is overkill, and it leaves me worried that this PR's current direction would result in too much complexity too early in the project's life. But I'd like to explain why I think that, suggest a different direction, and see what yall think!


Let's imagine the life of the create-next-app package.

The first version was published 5 years ago and its commit history seems to go on forever.

That's a lot of time and effort, lots of discussion, lots of decisions, lots of documentation (internal and external).

If we were to visualize its life, lets pretend it looks something like this:

image

It started small and as Vercel iterated, it grew into something much larger and more mature.

Now let's imagine our project's life:

image

It's just barely getting started right? So what happens if we merge in such a large amount of code from create-next-app?

image

It exploded in size!

That alone by itself isn't necessarily a problem (sometimes you get lucky and have a code-writing machine for a teammate!), but I think the real challenge comes about if we think about the size of the circles as "context needed to understand the codebase".

A large context requirement makes sense for create-next-app: the project has been in development for a long time, the team that built it has worked together for a long time, and the project has lots of internal and external documentation and "tribal knowledge" attached to it. There's a lot to understand in order to work on it, but that's entirely realistic for their team.

On the other hand, a large context requirement is a huge problem for us! We're all on the same page right now about what our project does and how it works because we're iterating on it together. That will remain true as long as we continue to iterate and build this thing piece-by-piece.

But if we inherit a large chunk of code from a project none of us have even been involved in, we'll find ourselves maintaining and trying to build on top of a system we don't understand.

We'd be like a bunch of mechanics trying to work on an engine none of us has ever seen and without a schematic! We could probably make it work, but it likely wouldn't be very enjoyable.


So instead of this approach, what could we do differently? What would allow us to size this PR's circle to fit naturally on our timeline?

image

I suggest that we start small and iterate!

How we could start small:

// probably not exact, but something like this
await execa('./node_modules/.bin/create-next-app', [
  '--example=https://github.com/developer-dao/create-dao',
  '--example-path=template',
  directory,
])

This is great because we get all of the power of create-next-app without having to take on the burden of maintaining their code or trying to build on top of something we don't understand. It's just a bit of code that we can all understand and build from.

And build from it we shall! This sets us up for an easy path to iterate in follow-up PRs:

Iterating in that way allows us to get the same benefits we're looking for, but in a way that keeps our new circles sized to fit the current project.


Hope that explanation made sense! Let me know what you think! ❤️

henripal commented 2 years ago

Yes, makes a ton of sense to me! I can take a stab at it tonight!

henripal commented 2 years ago

Didn't get the Thanksgiving commit done ;) here's a much, much simpler version of the code that does basically the same as the previous commit. Thanks so much @with-heart && @Dhaiwat10 for your thoughtful reviews. I'm taking notes :)

Dhaiwat10 commented 2 years ago

@henripal Ran yarn, npm i -g and then npx create-dao. I keep seeing this:

image

henripal commented 2 years ago

@Dhaiwat10 aa5f258 should fix this. prev version had the relative path to create-next-app which caused your error (initially wasn't able to reproduce bc I was testing from the root of the repo)

Dhaiwat10 commented 2 years ago

@henripal unfortunately, it still doesn't work for me. Tried on both my Apple and Windows machines.

$ npx create-dao

events.js:377
      throw er; // Unhandled 'error' event
      ^

Error: spawn create-next-app ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:274:19)
    at onErrorNT (internal/child_process.js:469:16)
    at processTicksAndRejections (internal/process/task_queues.js:82:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (internal/child_process.js:280:12)
    at onErrorNT (internal/child_process.js:469:16)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawn create-next-app',
  path: 'create-next-app',
  spawnargs: [
    '--example=https://github.com/Developer-DAO/create-dao/tree/main/template'
  ]
}
Dhaiwat10 commented 2 years ago

Can you try running it on a different machine too? Maybe there's some local dependency on your machine that we missed?

Dhaiwat10 commented 2 years ago

With the latest changes it works fine on Apple but not on Windows. Merging this for now and creating an issue for the Windows problem. Thanks for the PR @henripal :)