feat: use basepath name for project name and add on-create hook

[sor4chi]

Hi, @yusukebe

I implemented what we discussed in honojs/hono#1563 to try it out. What do you think?

[yusukebe]

@sor4chi,

Great work! That's exactly what I was looking for.

I've left some comments, so please check them.

And... do you think we can write tests? It would be better to have some, but I understand it might be difficult for this kind of CLI. I know there are some create-xxx CLIs for major frameworks that don't have tests.

[sor4chi]

@yusukebe

Thank you for the reviews. I resolved the all conversations.

About the tests, If you need me to do it, I'll do it.
But I think it's difficult too...

[raphaelkieling]

Hello!

Sorry for the intrusion, just a free time checking random PRs hehe! What about test only the core logic after-create.ts instead of a integration test in the cli itself (since is the hard part)? check if the templates hashmap is being populated correctly, if the hooks are being applied in the correct order and mainly if you are really replacing the correct place in the template file.

Seems important since the quantity of templates should grow with different logics, i can help you if you want. Thanks!

[yusukebe]

Hi @raphaelkieling,

No problem! I agree. @sor4chi, can you try it?

I think an integration test is hard because it has to create an actual project (but, it may be possible if we do proper cleanup?).

[sor4chi]

Hi, @raphaelkieling , @yusukebe

I tried to write some tests, how about this?

[yusukebe]

Commented. And please delete pnpm-lock.yaml .

[yusukebe]

Hi @sor4chi,

I tried to write some tests, how about this?

Thanks! I think it's almost good, but please give me a while. I'll reconsider this design once more (there may be no problem).

[yusukebe]

@sor4chi

What do you think creating a class for "Hook" like the following:

https://github.com/honojs/create-hono/blob/current-directory-name-class/src/hook.ts

This will be useful to make other hooks and make the code clean, I think.

[sor4chi]

@yusukebe
Cool, it sure looks like it would be easy to expand.
Surely, my code is redundant because I have to define addAfterCreateHook every time.

I will adopt the code as is. Thank you!

[yusukebe]

@sor4chi

Thanks. Use this PR! sor4chi#1

[sor4chi]

@yusukebe
By the way, is it ok to make hookMap public to test addHook?
or add a private prefix and access with ['field_name']?

[yusukebe]

I'd prefer not to. I think it's not needed if addHook is used in rewriteWranglerHook and will be tested accordingly.

[sor4chi]

Thank you. I understand!

[sor4chi]

I'm done!

[yusukebe]

I've left a comment.

[sor4chi]

Opps, i fixed!

[yusukebe]

Cool! Looks good!

@raphaelkieling do you have any comment?

[raphaelkieling]

@yusukebe @sor4chi awesome work, definitely a more resilient approach since would be really easy to extend for beforeCreate or any other necessary hook.

My suggestion for now or in near future is to create file a called hooks/cloudflare-workers.hooks.ts to avoid put specific template requirements inside the hooks/after-create.ts. With this, the hooks/cloudflare-workers.hooks.ts can centralize all the logic for afterCreate (or any other future hook) and the hooks/after-create.ts can centralize ONLY the function interfaces and the new Hook<...>().

What do you think?

[yusukebe]

Hi @raphaelkieling,

Thank you for sharing your suggestion! Your way is good for a separation of concerns. I think we should not do it now, but we can consider it in the near future when new hooks will be added. So, I'd like to go with this for now.

@sor4chi, what do you think?

[sor4chi]

@yusukebe @raphaelkieling

I think it's fine to release it once with this and sort it out if it becomes a problem, as I can add it later as well, and I don't think there's much benefit enough to manage it that neatly right now.

[yusukebe]

Okay! Let's ship it.