-
Notifications
You must be signed in to change notification settings - Fork 59
RFC: Support Luau-syntax configuration files #125
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
base: master
Are you sure you want to change the base?
Conversation
Looks pretty great. My bikeshed is that .luauconfig as a name is fine but I'd rather have .config.luau so that syntax highlighting just works and there's no need for more editor and especially github configuration. |
I've renamed For runtime configuration, I've introduced a new |
|
||
For analysis-time configuration extraction, we adopt the same behavior described in the [user-defined type functions RFC](https://rfcs.luau.org/user-defined-type-functions.html): we simply respect the time limit that the embedding context provides through the `Frontend` API. | ||
|
||
At runtime, the configuration extraction timeout is set to two seconds by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it 2 seconds per config file (my current understanding), or 2 seconds overall for config resolution? Given that the directory hierarchy matters and we'll potentially be extracting a config from every ancestor directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of going back and forth. My original intention was per-file, but now am unsure about a design that causes a linearly scaling hang time wrt. virtual filesystem depth.
At runtime, there's only one use case, which looks like this with a cumulative timeout: every time we see an alias in require
, we attempt to resolve it by executing each VFS node's .config.luau
(going up the ancestry chain as needed). If we reach the timeout limit while executing a .config.luau
in the chain, we throw a timeout error.
That seems okay to me. Ultimately, the timeout is a protective feature—I'd expect that extracting information from .config.luau
would be trivial in almost all cases.
There are a few ideas I'll toss out that I think could make this better.
|
However, if both `.luaurc` and `.config.luau` are present in a directory, only the `.config.luau` file is used. | ||
Configuration files are not merged; at most one configuration file is recognized per directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that to remain consistent with other similar cases with require
, this should be ambiguous and error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine.
An example `.config.luau` is given below. | ||
|
||
```luau | ||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add a special case in Frontend to add a constraint on the module's return type to be LuauConfig
like below. This enables autocomplete for free.
**Restrict syntax**: Limit `.config.luau` files to only contain table literals and forbid functions, loops, and other runtime constructs to reduce complexity. | ||
Instead of spawning a new Luau VM to "parse" the file, we could simply spin up a Luau parser. | ||
|
||
**Improve support for `.luaurc` files**: We already provide an autocomplete API for Luau-syntax files and could expose an API that facilitates editing `.luaurc` files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
I think this is a bad idea. Small interrupts on a machine will cause builds to be flaky.
This is best left to a separate proposal explaining the exact runtime semantics. |
These two are good calls IMO, and we should integrate both. |
Rendered