Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amended require resolution for init files #76

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jackdotink
Copy link
Contributor

@jackdotink jackdotink commented Nov 20, 2024

This proposes new semantics for requiring init files in folders to more closely align with the Roblox DataModel and many users' understanding of how init files behave.

Rendered.

@bradsharp
Copy link
Contributor

My concern with treating init.luau as if it were in the parent directory is that it introduces a major footgun to any new user of the language.

When a new user starts a project, they'll likely create a file called init as that would be the convention. At some point, that user introduces a second file called foo and they attempt to require it from init with require("./foo"). This will throw error as init is treated as the parent directory of which foo. not a child. So, they learn the correct way to require this is using a special alias such as : and rewrite their require to be require(":foo"). Later, they introduce a third file called bar and attempt to require it from foo with require(":bar"). This will throw another error as foo, unlike init is not treated as the parent directory.

I think we can find ways to mitigate this, but fundamentally I think "promoting" a file to the parent directory introduces more problems than it solves. We need to give this some more thought, and once we have a proposal we'll share it for feedback.

@jackdotink
Copy link
Contributor Author

This is a valid concern.

I think that if Luau were to start autocompleting imports this would be mostly negated. Additionally, I don't think this will confuse many users, as Rust does something similar and they experience minimal confusion - likely because they autocomplete imports.

I am personally not a huge fan of : as the symbol for these imports, and perhaps something like self/child would be more descriptive and help users understand. For new users in Roblox Studio, these semantics will cause little to no confusion because Roblox shows init files as the actual folders, but I can see how it would cause confusion on the filesystem.

@ffrostfall

This comment was marked as resolved.

@aatxe
Copy link
Contributor

aatxe commented Nov 20, 2024

I'm inclined to disagree about the footgun. Fundamentally, the way I think about this is that require-by-string is introducing module paths for the require system. We do not have a special syntax for these paths (yet? maybe we will next year when we work on import syntax), but they are not filesystem paths, and we should communicate that. If it weren't for the fact that lua already poisoned colons, I'd probably even have proposed using : as the separator instead /. The consequence of that view though is that I don't think it's particularly important to preserve filesystem-y behavior unilaterally (which seems to be what we're getting without this RFC: init.luau is treated as 'just another file' and so its siblings are whatever the filesystem says it is). Given that we live in a world where init.luau is already treated as special for modules by existing tools and the concept of nesting a script under a script is already alive and well (both because of Roblox but also because of how people are using lune as well), I think this flavor of proposal is the right way forward: we should accept that init.luau is the way to reify a module with a nested submodule below it onto the file system.

In this light, the structure that makes the most sense to me is to mirror how Rust deals with mod.rs. For those unfamiliar, you can write a module named foo either as foo.rs or foo/mod.rs. The latter naturally allows you to write submodules by the same rules, so a nested submodule foo::bar can exist at either foo/bar.rs or foo/bar/mod.rs. Copying the same example from the RFC, this is what I would propose, with two options for the syntax, retaining the choices we've already made for one, and backing out on some of them for the other:

- sibling
- folder/
  - init
  - module
  - subfolder/
    - init
	- submodule
From To Path
sibling folder/init ./folder or erroring and requiring something like pkg/folder
folder/init sibling ./sibling or erroring and requiring something like pkg/sibling
folder/init folder/module @self/module (using alias syntax) or self/module
folder/init folder/subfolder/init @self/subfolder or self/subfolder
folder/init folder/subfolder/submodule self/subfolder/submodule
folder/module folder/init .. or parent
folder/module folder/subfolder/init ./subfolder or subfolder
folder/subfolder/init sibling ../../sibling or pkg/sibling
folder/subfolder/init folder/init .. or parent/folder
folder/subfolder/init folder/module ./module
folder/subfolder/init folder/subfolder/submodule @self/submodule or self/submodule
folder/subfolder/submodule folder/subfolder/init .. or parent
folder/subfolder/submodule folder/init ../.. or parent/parent

Not married to any of the specific syntax really, but the overall semantics is what I think makes the most sense, really. The main variation that is sensible (and analogous to what Rust does, but people may find annoying) would be to make it so that sibling modules cannot be required directly and that the path has to go through the parent to do it.

@Dekkonot
Copy link
Contributor

I feel as though the summary of this RFC should be expanded to be an actual summary of the changes, not just describe what the RFC does.

Otherwise, I'm inclined to agree with this approach. Rojo wouldn't need to change (at least at a glance) and it solves the problem. These tables are very confusing and I think we should probably just commit to including a proper support for self in the require syntax; having it be an alias is just weird, because those should be implementor- or user-defined IMO. Otherwise, it's good that we're deciding that string requires aren't actually file paths but instead just happen to look like them.

It was brought up on Discord that we could perhaps add support for self by making it look like a URI? e.g. self://child instead of self/child. I think I'm throwing my support behind that if we end up with a special thing for it.

@aatxe
Copy link
Contributor

aatxe commented Nov 20, 2024

Yeah, I talked about it with @bradsharp over lunch too, and I'm increasingly convinced that the URI-style is a nice way to do it.

@dphfox
Copy link

dphfox commented Nov 23, 2024

I talked to some less terminally-engaged-in-RFC devs since we've been talking about what laypeople would find intuitive, and I learned a lot and heard some good ideas from them. As a result, I've changed my mind from the other PR.

Fundamentally, the way I think about this is that require-by-string is introducing module paths for the require system. We do not have a special syntax for these paths (yet? maybe we will next year when we work on import syntax), but they are not filesystem paths, and we should communicate that.

100% agree. Module paths should be unopinionated about representation - that's the platonic ideal we should be working towards. In an ideal world, we do not need to care about whether something is in a filesystem structure, or embedded in a program, especially as Luau is adopted as an embedded language in other engines.

we should accept that init.luau is the way to reify a module with a nested submodule below it onto the file system.

I don't think reification is strictly necessary. Backwards compatibility absolutely is, and so I think we should support this, but I think there's also room for something a little better fit for everyone's needs together.

In particular, I got the impression from talking with less technical devs that they do not like the init.luau convention and that the footgun the Luau team are concerned about is a valid, material concern. I think we should do something about that.

One dev suggested something really simple: "why not just have a file and folder with the same name?". For catchiness, I'll call this the "file + directory" approach.

- myModule.luau
- myModule
    - child.luau
- sibling.luau

The reason I like this is that the parent module is always one level above the child module. Thus, when syncing between an embedded and filesystem representation, no special casing needs to be done.

One concern I heard about this was that the folder and file may appear distant from each other in the file explorer, especially on Windows which shows folders in a block at the top of the list, before all files. Happily, it looks like major editors have features for supporting file nesting of exactly this variety, so I think this shouldn't be a big deal.

@self/subfolder

At first, I didn't really like these contextual aliases, but maybe they're OK.

As an alternative, since paths are relative to the parent, you could also use the name of the module directly:

myModule/child

though of course this doesn't work quite as neatly for long names.


So overall, here's where my opinion lands:

  • Acknowledge and support init.luau files for backwards compatibility.
  • Introduce file + directory as the recommended way forward for new projects.
  • Preserve ./ and ../ behaviour. Perhaps use @self contextual alias if deemed necessary.

This should amortise the cost of fixing this issue over a longer time period, and means that projects don't have to hard-switch to a new syntax all at once. I personally believe this to be an optimal approach because it means that we can focus on preserving backwards compatibility fully, without encumbering non-Rojo Luau users with a myriad of confusing behaviour.

The only downside I can see is that there are ambiguities introduced by supporting both, e.g.:

- myModule.luau
- myModule
    - init.luau
    - init
        - init.luau 

But given the impassioned nature of this debate, this is perhaps just a cost of doing business.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants