rescript-lang / rescript-compiler

The compiler for ReScript.
https://rescript-lang.org
Other
6.73k stars 447 forks source link

allow filenames like (404.ml, multiple index.ml in a project)(Issues with gatsby and filename restrictions) #1974

Closed glennsl closed 6 years ago

glennsl commented 7 years ago

I'm working on getting gatsby to work with bucklescript+reason-react and have a small starter project up an going at https://github.com/glennsl/reasonable-gatsby-starter. I've run into some issues however, and all the major ones are related to filename restrictions, since Gatsby relies quite heavily on the file and directory structure of the project. Here are some of the issues, workarounds and possible solutions I've found:

  1. Some features require files to be have a specific name, like 404.js, which is of course not a valid OCaml module name.

The workaround is to rename the .re to something valid (error404.re) then add a stand-alone 404.js that simply re-exports error404. It's messy, but it works.

Is it possible to add some sort of name mangling to the module name generation?

  1. This is a big one. Gatsby generates URLs based on the pathname of each page-specific .js-file. Since filenames must be unique, it is not practically possible (or at least very inconvenient) to have url segments with the same name. It also causes other conflicts where special filenames are needed. In particular, it is not possible to directly have multiple index pages in different directories.

The workaround here is the same as above, rename the reason file and add a stand-alone js-file with the correct name that re-exports the generated js of the renamed file.

Is it possible to have an option to enable the generation of module names based on the path? So we could have pages/index.js and layouts/index.js generate the module names Pages__Index and Layouts__Index respectively.

  1. Restrictions on filenames will of course also lead to restrictions on possible URLs. In particular, URLs with dashes aren't possible. This is a relatively minor issue, and while it would probably be possible to resolve it with name mangling it's probably not worth it. Still, it's something to consider. The workaround for this has simply been to use underscore instead of a dash, which is fine, really.
bobzhang commented 7 years ago

we can do this in the build system level instead of relaxing names?

glennsl commented 7 years ago

It seems it would have to involve code generation too, to "unmangle" the module names for imports and such, wouldn't it? Build system can take pages/index.re and generate Pages__index.cm*, but code generation would still have to emit pages/index.js and require('../pages/index.js').

I guess it could be done with some post-processing by the build system.

bobzhang commented 7 years ago

I mean you still generate pages.js but allowing a field to copy it, something like below

"copy" : [ [ "page" , "index"] , ["error", "404"], ["layouts", "index"]]
glennsl commented 7 years ago

I'm not sure I follow. So you'd have pages/pages_index.re (or something else that's unique), which produces a module called Pages_index and emits js to pages/pages_index.js. Then the "copy" config means it should copy that to pages/index.js?

bobzhang commented 7 years ago

@glennsl yes, that's something I have in mind. would copy be good enough or do you need remove page_index.js in practice?

glennsl commented 7 years ago

@bobzhang It would of course result in unwanted pages being generated, and would also slow down the build process (a big part of gatsby DX is hot module replacement, and it's unfortunately already pretty slow). But none of these issues are dealbreakers I think.

A bigger concern is how the configuration would work. I don't fully understand the config example you posted above, but it looks like it's per-item, which would quickly get unmanageable and error-prone.

d4h0 commented 5 years ago

@glennsl: Did the problem get fixed or did you find a solution? Otherwise, why did you close this issue? I think, some kind of workaround still would be useful.

hodatorabi commented 4 years ago

@glennsl Hi. I'm currently having the same issue with nextjs + reason. Did you find a workaround for this?