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

RFC: Absolute Require Syntax #80

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

bradsharp
Copy link
Contributor

This RFC proposes additional syntax for require by string that allows modules to be resolved relative to the root of a project or library. It is based on #78 which proposed the same changes but also removed relative paths.

Rendered.

@vegorov-rbx
Copy link
Collaborator

This really sounds how we wanted to have libraries to work initially.
We explored both having an alias path '@' to point to the root of the library as well as just having library root '.' in paths inside luaurc to resolve non-relative requires from module root. That was before we removed absolute requires.
This really sounds like the later, but codified implicitly without a need to place '.' in each library luaurc file.
Sounds good and I don't think it even interacts with relative requires, so any problems with that can still be fixed separately if desired.


## Motivation

It should be possible for all users of Luau to write code that is capable of running, without modification, on many different platforms. Unfortunately, we identified an issue with the current syntax that would make it difficult for users of certain tools to adopt. As paths are relative to the file calling require, any tools that adjust the file hierarchy would also need to update require paths to match. While we know there may not be a perfect solution that works for all use-cases, we think the current syntax should be amended to support these cases or at least not require them to translate require paths in files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, we identified an issue with the current syntax that would make it difficult for users of certain tools to adopt.

Which tools?

As paths are relative to the file calling require, any tools that adjust the file hierarchy would also need to update require paths to match.

What tools would adjust the file hierarchy other than a human moving files around?

This can be solved with code refactoring tools. In TypeScript, if you have some statement

import ... from "./foo"

You can press F2 on "./foo" and it will rename the file, and also applies renaming over other imports that resolves to the same file, ditto any kind of action of moving files around as long as the action of moving files around happens through LSP.

I'm not a fan of any RFCs that wants to cop out from motivating the need for better tooling, similar to postfix ! to remove nil, and this RFC feels like it has the same problem.


## Alternatives

We have already considered (and even approved) some alternatives to this approach, however there are drawbacks with each of them that ultimately led us in this direction.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I see this section reworked? As it is, it requires you to read all the RFCs and filtering out irrelevant information wrt what this RFC is trying to solve. It's also not really "alternatives" either.

@alexmccord
Copy link
Contributor

alexmccord commented Nov 29, 2024

Had to read all the related RFCs that was spawned in the last week surrounding init.luau and why it was problematic. This comment explained the problem the best, and gave me a lot of context about why we're giving this issue any attention. Decided to collate all this information so that I can look at this problem clearly and what I think the resolution is.

Given this filesystem:

foo.luau
mod/
  foo.luau
  init.luau

With such contents:

% cat foo.luau
local m1 = require("./mod/init")
local m2 = require("./mod")
print(m1 == m2)
% cat mod/foo.luau
local m = {}
function m.id(x) return x end
return m
% cat mod/init.luau
return require("./foo")

Piping this through Rojo gives this DataModel structure:

foo
mod <- ModuleScript whose source is init.luau
  foo

To make matters worse, there is supposed to be a bijectivity between the require by string and require by DM paths:

Prefix by string Prefix by DM path
./ script
../ script.Parent
@mod/ game:GetService("ServerScriptService"):FindFirstChild("mod")

This presents a counterexample: require(script.foo) != require("./foo") due to Rojo's flattening, which implicitly causes ./ in the DM to actually function as ../ in the filesystem.

Consequently ./ in init.luau is fixed relative to the directory that contains mod instead of mod folder itself. I can now see how the bijectivity property between the filesystem and the DataModel are broken, and that it is caused by Rojo flattening a folder with init.luau into a ModuleScript that takes the content of init.luau.

Following this line of thinking, you can see another diverging behavior in the other direction: foo.luau cannot require mod/init.luau because mod/init.luau does not exist in the DataModel, due to Rojo flattening this structure.

In Lune, if you have the above filesystem structure and its contents and you run foo.luau, it prints true. Running this through the DataModel, you get an error saying that foo.luau could not require mod/init.luau because it doesn't exist.

This leads me to conclude that it might have been a benign design bug in Rojo's mapping between filesystem <-> DM. I say benign because it has always existed and was never shown to be a problem until now, just like if x == nill then y else z with the intent of checking if x == nil: it was not a problem until Luau got a type system that flags for unknown globals like nill. The same can be said here too, Rojo's mapping feature was designed for that world where require by string did not exist, but that doesn't mean Rojo can't change.

The question I have now: is this a language design issue, or is this an issue elsewhere? I don't want to want to jump to the conclusion about whose issue it is, but it doesn't feel like a language issue to me.

Perspective: Luau needs to change something

If you adopt the view that the language has to change to fix the problem, you get these RFCs:

  1. RFC 76 proposes to redefine ./ in any init.luau to work one level up in the filesystem and adds a new prefix (self or script or whatever) whose semantics are equivalent to require(script.foo).
  2. URI style strings which is pre-RFC, but it takes the view that the language has to change to fix the problem.
  3. File + directory is essentially the same as RFC 76, only that init.luau has the same name as the directory, and sits as sibling to the directory.
  4. RFC 78 proposed to add absolute paths relative to the root directory, and also proposed to remove relative paths. A nuclear RFC.
  5. RFC 79 proposes to basically freeze all changes to require by strings. A nuclear RFC.
  6. RFC 80 proposes the same as RFC 78, minus the proposal to remove relative paths. Also a nuclear RFC, but to a lesser degree than the previous two.

The last 3 RFCs listed here got me looking at this and think about what I could do to step in and defuse the situation.

My stance is that none of the above fixes the problem where the filesystem and the DM are observationally different.

Perspective: Rojo needs to change something

If you shift perspective and think that Rojo should change to fix the problem, you will be able to see better progression towards a real resolution by adding a constraint where the filesystem and the DM must be structure preserving to maintain bijection.

Currently, it conflates flattening with mapping, where a folder containing init.luau is flattened into a ModuleScript without init.luau. This is what causes the relative prefixes to work one level up in the DM as opposed to the intended level.

By removing the flattening part out of Rojo, you end up with this structure, exactly equivalent to one in the filesystem.

foo
mod <- Folder instance
  foo
  init

Way better.

Indeed, we have two issues that arises from doing this:

  1. Require by DM paths can no longer require mod itself. I consider this to be a missing feature on Roblox's behalf.
  2. Fixes relative require by strings using ./ and ../, but breaks relative requires by script. This one appears paradoxical, but can also be resolved on Roblox's behalf.

This means Rojo was bludgeoned into a corner, and only one way out left: Roblox.

Perspective: Roblox needs to change something

If you again shift perspective and think that Roblox is missing something, then you can ponder why flattening was added in Rojo in the first place.

The reason was purely a syntactic convenience. Without it, you had to settle on a convention on which script was the entry point for a collection of modules. You would also have to deal with one extra .Parent in DM paths that were already verbose to begin with. This motivates Rojo to want some way to rearrange the DM for your convenience.

This boils down to a missing instance type, call it Library. It has the following interactions:

  1. If require resolves to a Library instance, resolve to the init script within the library. Both require by string and require by DM paths should do this for Library instances.
  2. A ModuleScript named init that is a child of Library has script bound to the Library instance itself, rather than the init ModuleScript.

This effectively does the flattening for Rojo, without making flattening a destructive operation done by Rojo, allowing Rojo a full and faithful functor to represent the original filesystem used to map over into Roblox for the purpose of resolving requires without any observable differences.

With a Library instance, we'd end up with this:

foo
mod <- Library instance
  foo
  init

Rojo no longer has to flatten this out, since Roblox supports it first-class, and restores all backward incompatibilities that was not resolvable with Folder. Only at this point can Rojo remove the flattening and change how folders with init.luau are mapped over to the DM.

This feature exists with require(id) using MainModule as the entry point, but does not exist in the DM paths variant. This isn't anything unique, even require by string has this feature. It's the require by DM paths that's the oddball here.

First shot

NB: This is also in draft. The suggestion here does solve the problem, but does not solve the problem in general because I've missed or glossed over details.

I think we should take this issue as a growing pain towards having a Library instance whose purpose is to represent what the filesystem looks like in a structure preserving way. It should get prioritized, and as soon as it is released, Rojo can switch over to that, resolving the require by string inconsistency once and for all.

Until such an instance exist, Rojo is literally incapable of doing anything. It can't change the way it maps the filesystem to the DM without breaking one require style or the other. This means that require by string and require by DM paths are mutually exclusive to each other if you're using Rojo.

Libraries gives Rojo a way out of this mutual exclusion problem.

Then it becomes obvious that all RFCs proposing to change the language can be thrown out, closed, and forgotten about. All of this just stems from the lack of confluence in semantics between each style of requires, which needed to be resolved.


My point I want to get across is that some of the proposals (RFC 78 and 80) feels like a knee-jerk reaction to resolve the problem today at the cost of sacrificing the design principles that was used to develop require by string RFC, which explains why RFC 79 was submitted in the first place.

@jackdotink
Copy link
Contributor

The idea that Roblox could change to support the filesystem is indeed a nice idea, but you also have to consider the users. To some extent you can rely on the "if we build it they will come" mentality, but you have to remember this is a convention that has existed in Roblox for over a decade.

Is it really worth breaking that convention, and the way Rojo syncs projects (which will make string requires unusable without significant modification to existing Rojo projects), to better align with the file system? I don't think that it is.

I think a better idea is to change the way we think about the file system to instead match how the datamodel functions. This is helpful for Rojo, as it won't have to change, but also helpful for Roblox when they create their own tooling to put projects in the filesystem.

@alexmccord
Copy link
Contributor

alexmccord commented Dec 2, 2024

Yeah, I checked in with Dekkonot and when you add in a few other edge cases that Rojo has to account for, the libraries suggestion is not enough, and there's no good solution other than to give up the view that the DM could model the filesystem faithfully. I'm beginning to think that URIs are a good idea as long as it doesn't interact with the language design, because this is still a Roblox-ism.

I had hoped that Rojo could change the way it maps over to the DM and still be backward compatible with the older version of Rojo in the DM, but I do recognize that requires extra code that amounts to doing a database migration. I also glossed over the fact that users might actually care about the DM structure that Rojo outputs to, even to the degree that mod folder has to be a ModuleScript (although I adopt the view that would be a user problem...)

@vegorov-rbx
Copy link
Collaborator

  1. Require by DM paths can no longer require mod itself. I consider this to be a missing feature on Roblox's behalf.

This is not correct, requiring a Folder instance will require 'folder/init' module in the current Roblox implementation.

Changing the value of script is likely not possible, for internal reasons.

@alexmccord
Copy link
Contributor

alexmccord commented Dec 2, 2024

This is not correct, requiring a Folder instance will require 'folder/init' module in the current Roblox implementation.

This works using require by string. Not by DM paths, which was what the sentence says.

@vegorov-rbx
Copy link
Collaborator

This works using require by string. Not by DM paths, which was what the sentence says.

Thanks for clearing that up, I incorrectly assumed it talked about string path in a DM.

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