gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
18.07k stars 757 forks source link

Permit FFI modules in subdirectories #1562

Closed lpil closed 13 hours ago

lpil commented 2 years ago

i.e. src/one/two/three.js and src/one/two/three.erl.

We will need to track Erlang module names to ensure there are no clashes. For example src/one/themodule.erl and src/two/themodule.erl in one project is an error.

hayleigh-dot-dev commented 11 months ago

Adding some extra context at @erikareads (on discord)'s request:

You can only write ffi code directly inside src/, but we'd find it a lot more useful if i could co-locate ffi files along with the modules that need it:

src/
 wibble/
  wobble.gleam
  wobble_ffi.mjs

It'd mean inside wobble.gleam we could write

@external(javascript, "./wobble_ffi.mjs", "thing")

instead of

@external(javascript, "../wobble_ffi.mjs", "thing")

and so on.


For a concrete use-case lustre has a lustre/try.gleam module that spins up a http server for either erlang or js and there is some ffi code to do that in http_ffi.erl and http.ffi.mjs accordingly. That ffi code is not relevant to the rest of the codebase so keeping it in the project root places undue importance on the code and also makes it minorly annoying to import from.

As lustre gains more CLI commands more of these one-off FFI modules are likely to be necessary so it'd be really great if they could be organised away in a way that makes more sense than how it is currently.

erikareads commented 11 months ago

For Javascript this seems to be the path creation function:

https://github.com/gleam-lang/gleam/blob/4396f9a38a2439d5c7961f952adeec93bd9db624/compiler-core/src/javascript.rs#L324-L341

It currently assumes that it will be in the top of the project.

erikareads commented 11 months ago

There seems to be a similar function in the typescript.rs file:

https://github.com/gleam-lang/gleam/blob/4396f9a38a2439d5c7961f952adeec93bd9db624/compiler-core/src/javascript/typescript.rs#L279-L297

winterqt commented 8 months ago

Has anyone tried to take a stab at implementing this yet? If not, I'd like to give it a shot, I just don't want to step on anyone's toes.

lpil commented 8 months ago

It's all yours @winterqt !!

isaacharrisholt commented 5 months ago

For the Erlang name clashing, is there a reason you couldn't use an absolute name, like the compiled Gleam does anyway? e.g.

src/
 wibble/
  wobble.gleam
  wobble_ffi.erl

And the compiled output is:

wibble@wobble.beam
wibble@wobble_ffi.beam

That would avoid that problem, and then you'd only have to worry about naming a Gleam module the same as an Erlang one (which would be an issue either way, and would be an issue on the JS target too)

lpil commented 5 months ago

No, we will not modify the Erlang source code in any way. We do not want to create our own slightly modified version of Erlang, the language should be unchanged whether or not the Gleam build tool is used.

PgBiel commented 2 months ago

We will need to track Erlang module names to ensure there are no clashes. For example src/one/themodule.erl and src/two/themodule.erl in one project is an error.

Question (as someone not too experienced with the Erlang target's inner workings): is this still a problem if the two files declare modules with different names (and thus parsing the files would be needed to avoid clashes)? Or does Gleam force the modules to have the same names as the files they're in?

It's also worth mentioning that I've observed that, on the JS target, you can have two files like abc.gleam and abc.mjs and the latter will override the former without any warnings. This might just be something else that needs improvement though (thus unrelated to the Erlang problem).

lpil commented 2 months ago

is this still a problem if the two files declare modules with different names (and thus parsing the files would be needed to avoid clashes)?

You don't need to parse the module to work out the name, the name is always the same as the file name. The directory name is not used in any way for Erlang modules.

on the JS target, you can have two files like abc.gleam and abc.mjs and the latter will override the former without any warnings. This might just be something else that needs improvement though (thus unrelated to the Erlang problem).

If we don't prevent this from happening in the PR let's open an issue to make sure we prevent this in future.

PgBiel commented 2 months ago

is this still a problem if the two files declare modules with different names (and thus parsing the files would be needed to avoid clashes)?

You don't need to parse the module to work out the name, the name is always the same as the file name. The directory name is not used in any way for Erlang modules.

Okay thank you, I might be able to add this to my PR then. Though, is this also true for .ex files?

on the JS target, you can have two files like abc.gleam and abc.mjs and the latter will override the former without any warnings. This might just be something else that needs improvement though (thus unrelated to the Erlang problem).

If we don't prevent this from happening in the PR let's open an issue to make sure we prevent this in future.

Good idea. I'll see if the PR would be able to fix this, if not I'll open an issue.

lpil commented 2 months ago

Though, is this also true for .ex files?

Elixir has no relationship at all between file names and module names so we can't predict that modules a file will produce.

PgBiel commented 2 months ago

Ah bummer. I guess we can't really give a helpful error in that case and just let it error on the Erlang build step instead, right?

Though I guess that isn't that bad, as the user would be intentionally doing so. If Elixir automatically joins the modules, that might be even less of a problem.

lpil commented 2 months ago

I think we'll just have to leave it up to the programmer to manage that for now.

PgBiel commented 2 months ago

I was going to add the duplicate .gleam and .mjs module check to my PR, but before that I integrated a duplicate .gleam and .erl check to the duplicate Erlang module check (i.e. abc.gleam and abc.erl will error as well as a duplicate abc.erl somewhere else in the tree) - this check seemed needed, as the opposite problem happened (the .gleam file would always override the .erl file). The result was that Hex packages would always trigger an error as they come with precompiled .erl files inside the src/ folder for each .gleam file there, triggering false positives. Not sure how to best detect that .erl files come from Hex, but this doesn't seem trivial at first. We could open an issue and leave that to a future PR.

With that said, since that's not the case for .mjs files, my PR is now successfully detecting clashes between .gleam and .mjs (just needs a bit of cleanup).