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: Import syntax #1013

Closed
wants to merge 17 commits into from
Closed

RFC: Import syntax #1013

wants to merge 17 commits into from

Conversation

dphfox
Copy link

@dphfox dphfox commented Aug 19, 2023

Improve ergonomics of importing modules and open room for extended importing features by introducing an import statement, to replace a majority of require() calls in Luau codebases while respecting backwards compatibility concerns.

Rendered

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

This is my first time going through this RFC process, so let me know if there's anything else I need to do. Thanks :)

@Dekkonot
Copy link
Contributor

I disagree on merit with the idea of !import as syntax, because it is distinctly un-Luau-like. Additionally, every symbol reserved in this manner is one we can't use later, and there's only so many ascii symbols that Luau doesn't already use.

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

I disagree on merit with the idea of !import as syntax, because it is distinctly un-Luau-like. Additionally, every symbol reserved in this manner is one we can't use later, and there's only so many ascii symbols that Luau doesn't already use.

This is fair, but removing the symbol makes it super easy to run into ambiguity with existing call syntax:

local function import(x)
  -- ...
end

import "foo" -- valid Lua 5.1

I am open to further ideas on the syntax. I agree it is not optimal.

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

A syntax I've seen proposed on the OSS Discord (introduces extra contextual keyword from):

import from "foo/bar/baz"
import type from "foo/bar/baz"
import local from "foo/bar/baz"
import not_baz = from "foo/bar/baz"
import not_baz = thing1, type thing2, local thing3 from "foo/bar/baz"

It seems clunky with renaming.

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

It seems clunky with renaming.

Perhaps we can instead reuse the :: operator;

import from "foo/bar/baz"
import type from "foo/bar/baz"
import local from "foo/bar/baz"
import from "foo/bar/baz" :: not_baz
import thing1, type thing2, local thing3 from "foo/bar/baz" :: not_baz

@Kampfkarren
Copy link
Contributor

Kampfkarren commented Aug 19, 2023

Another issue is we now have a footgun where adding any new export is a breaking change.

-- (I don't remember what the global syntax you wrote is)
--!import Libraries.ModuleA
--!import Libraries.ModuleB

Any new exports in ModuleA break what you're using from ModuleB.

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

It seems clunky with renaming.

Perhaps we can instead reuse the :: operator;

import from "foo/bar/baz"
import type from "foo/bar/baz"
import local from "foo/bar/baz"
import from "foo/bar/baz" :: not_baz
import thing1, type thing2, local thing3 from "foo/bar/baz" :: not_baz

Updated the RFC to reflect this less objectionable syntax after discussion on Discord.

I realise this is ambiguous wrt the expression passed in as the module path. I have swapped it for = which is not ambiguous.

@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

Worth noting how this interplays with destructuring (#617):

With the current destructuring RFC, some questions arise wrt how exported types interact with destructuring. Specifically:

-- foo/bar/baz.luau

export type A = "something"

return {
  value1 = "something",
  value2 = "something"
}
-- somewhere else

local { .value1, .value2 } = require("foo/bar/baz")

local variable: baz.A -- is this allowed?

The more consistent answer is that this is allowed because destructuring is always equivalent to doing the destructuring manually, i.e. the above snippet should always be equivalent to:

local baz = require("foo/bar/baz")
local value1, value2 = baz.value1, baz.value2
baz = nil

local variable: baz.A -- of course this is allowed

However, the more useful answer is that this is disallowed because it likely pollutes the namespace, not to mention we are not naming baz itself anywhere.

local value1, value2 = require("foo/bar/baz").value1, require("foo/bar/baz").value2
local variable: baz.A -- of course this isn't allowed

I tend to agree with the latter over the former but this is a confusing state of affairs.

Secondly, destructuring makes working with types awkward. There does not currently exist a way to destructure the types exported from a module - the closest we have is type smuggling (as demo'd in the RFC) and this does not work with exported types, only with the types of values included in the module.

In theory, maybe it'd look like this? The require() type declaration stuff doesn't exist yet though, which is a problem.

type { .A, .B } = require("foo/bar/baz")

It also means we're duplicating the require statement twice if we want values AND types:

local { .value1, .value2 } = require("foo/bar/baz")
type { .A, .B } = require("foo/bar/baz")

It also does not provision for any kind of bulk destructuring as motivated to be relevant in this RFC. Perhaps that'd look like this, but this also does not exist yet:

local { ... } = require("foo/bar/baz")
type { ... } = require("foo/bar/baz")

I don't see how we could solve the duplication here trivially. That'd require more thought. I'm also concerned we might accidentally run into ambiguity territory with type {} also (esp since type is a builtin function)

I think it is sensible to want to create a dedicated import/export system here, since this is a specialised enough and common enough use case that already has 'magic' type behaviour and which already causes headaches. I can accept, however, that destructuring does at least get some of the way there, and I do not wish to shut it down as an alternative.

@dphfox dphfox changed the title RFC: Static import syntax RFC: Import syntax Aug 19, 2023
@dphfox
Copy link
Author

dphfox commented Aug 19, 2023

To address some clarity/conciseness concerns and to better mirror other environments such as Rust, JS, etc, I have replaced the local syntax with a more conventional and consistent asterisk * syntax.

@Hexcede
Copy link

Hexcede commented Aug 20, 2023

I would argue for something other than *, in particular, ... as it is more luau-like. * is (sometimes) ambiguous syntax since it makes a valid expression. Not sure if that actually matters though.

To fix pretty much all the problems mentioned I think that it would make sense to always require an identifier (or list of identifiers) after import (Same reason that export type works and allows extending the syntax after)

P.s. though technically outside the scope of the RFC, possibly take into consideration the potential for extended export syntax by making this syntax structure a pattern. This also seems to have close ties with table destructuring, #629 which I have notes on, but I will provide my comments there separately.

In practice:

-- Default import
import Fruits "./fruits"
import Veggies "./veggies"

-- Field imports
import Potato, Carrot from "./veggies"
assert(Potato == Veggies.Potato)
assert(Carrot == Veggies.Carrot)

-- Wildcard imports
import ... from "./fruits"
import type ... from "./fruits"
assert(Apple ~= nil)

-- (Apple type is defined)
local myApple: Apple = Apple.new()

-- Name overrides
import Pickle as PickledCucumber from "./veggies"
assert(PickledCucumber == Veggies.Pickle)

Since it's not in the scope of this RFC I left this stuff for last.

Exporting

  1. Have export syntax which is consistent with type exports and is only active for modules with no return statement (which would be invalid syntax for modules)
  2. Possibly provide an exports global (can absolutely be treated as a local) in this mode
  3. Exports can then be thought of as syntactic sugar for modifying some exports table.
-- Exports are consistent with imports
local banana: Banana = Banana.new()

export banana -- Exports the banana variable with the string key "banana"
export banana as aVeryGlobalBanana -- Exports the bana variable with the string key "aVeryGlobalBanana"

@dphfox
Copy link
Author

dphfox commented Aug 21, 2023

I would argue for something other than *, in particular, ... as it is more luau-like. * is (sometimes) ambiguous syntax since it makes a valid expression. Not sure if that actually matters though.

To fix pretty much all the problems mentioned I think that it would make sense to always require an identifier (or list of identifiers) after import (Same reason that export type works and allows extending the syntax after)

P.s. though technically outside the scope of the RFC, possibly take into consideration the potential for extended export syntax by making this syntax structure a pattern. This also seems to have close ties with table destructuring, #629 which I have notes on, but I will provide my comments there separately.

In practice:

-- Default import
import Fruits "./fruits"
import Veggies "./veggies"

-- Field imports
import Potato, Carrot from "./veggies"
assert(Potato == Veggies.Potato)
assert(Carrot == Veggies.Carrot)

-- Wildcard imports
import ... from "./fruits"
import type ... from "./fruits"
assert(Apple ~= nil)

-- (Apple type is defined)
local myApple: Apple = Apple.new()

-- Name overrides
import Pickle as PickledCucumber from "./veggies"
assert(PickledCucumber == Veggies.Pickle)

Since it's not in the scope of this RFC I left this stuff for last.

Exporting

  1. Have export syntax which is consistent with type exports and is only active for modules with no return statement (which would be invalid syntax for modules)
  2. Possibly provide an exports global (can absolutely be treated as a local) in this mode
  3. Exports can then be thought of as syntactic sugar for modifying some exports table.
-- Exports are consistent with imports
local banana: Banana = Banana.new()

export banana -- Exports the banana variable with the string key "banana"
export banana as aVeryGlobalBanana -- Exports the bana variable with the string key "aVeryGlobalBanana"

I'm not sure if introducing an exports table is completely necessary here, but I would certainly not be against aligning the way values are exported with the way types are exported. Unsure of how necessary it is for this RFC specifically since it doesn't necessarily address the motivations but it'd be nice for sure.

By the way, if an export statement were to be adopted, it should strictly accept identifiers, but also be declarable on local declarations. This would make it possible to export expressions directly without leaving them unnamed and without introducing ambiguity when exporting a string or table literal.

export local foo = "bar"
export "bar" - - ambiguous, unnamed

Ditto for local function declarations for consistency. Just thoughts.

@bradsharp
Copy link
Collaborator

Have you read the require-by-string RFC located here: #969?

@dphfox
Copy link
Author

dphfox commented Sep 14, 2023

Yes - I looked into it to ensure this RFC was compatible :)

@andyfriesen andyfriesen added the rfc Language change proposal label Sep 19, 2023
@zeux
Copy link
Collaborator

zeux commented Oct 30, 2023

This PR is closed as part of RFC migration; please see #1074 (comment) for context.

Additional notes: well, this is clearly controversial judging by the discussion and the reactions :) unsure to what extent this represents the original syntax. I think this RFC has a very large scope right now - much too large. Some parts I think are just completely off the table, such as star imports - it's important that every import is qualified with a local variable for a variety of semantical reasons, let alone ease of reading code. Some parts have somewhat unclear value to me. Additionally this is currently an area that is being evolved as far as string require RFCs are concerned; I think it's important that that work concludes before we offer further syntax/semantics improvements in this area. So it feels to me like while first-class imports/exports are likely inevitable at some point in long term future, we need to focus on finishing require specification and implementation, gain some field experience with it, and then consider what we do from there on. I would really try to look for cases where we can add significant value over nicer syntax first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Language change proposal
Development

Successfully merging this pull request may close these issues.

7 participants