dmmulroy / create-melange-app

The fastest, friendliest, and most delightful way to get started with OCaml, ReasonML, and Melange, geared towards JavaScript and TypeScript developers
104 stars 19 forks source link

Including '/' in a project name causes an unclear error #98

Open yashveer-bika opened 3 months ago

yashveer-bika commented 3 months ago

I have Node.js v21.7.1. The issue is shown below. I have opam installed and initialized. I tried bun and pnpm as well, got the same error.

Am I missing something?

Screenshot 2024-04-06 at 11 10 37 PM Screenshot 2024-04-06 at 11 10 51 PM
dmmulroy commented 3 months ago

I believe the issue is that there is / in the project directory. I just tried doing foo/bar and it failed for me with the same error, however using foobar worked fine.

dmmulroy commented 3 months ago

We should improve the regex validation here to prevent / in the project name: https://github.com/dmmulroy/create-melange-app/blob/develop/src/core/validation.ml#L18-L29

dmmulroy commented 3 months ago

[a-z_0-9.] I think we need to escape the .

JohnBakhmat commented 3 months ago

I checked the current regex on regex101, test case of bekka/ui should not be possible.
https://regex101.com/r/F43nZc/1 Looking into what can cause this bug now

JohnBakhmat commented 3 months ago

I figured that regex doesn't check for / because before project name is passed into validate function, a function parse_project_name_and_dir in src/core/fs.ml trims bekka/ and all we are left is ui, which is a valid name. Why it breaks after it tho 🤔. Probably it rans into mkdir issue when it doesnt create dir recursively

dmmulroy commented 3 months ago

https://github.com/dmmulroy/create-melange-app/pull/99#issuecomment-2044895275