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
Support maprecords #591
Comments
This should make __index metamethod handling work for dot notation as well (see #591).
See #591. This is not fully equivalent to a "maprecord" because we don't yet implement support for __newindex. But at least now the behavior of dot and index notation are consistent.
@uramer Thanks for the feedback! I've pushed a pair of commits that make dot-notation for |
Thanks, very cool. By the way, would maprecords be a reasonable first contribution to look into? And would such an MR be accepted, assuming the implementation is good? |
I am a bit reluctant about the concept of maprecords for two reasons: 1. Conflicts between record fields and maps keys of {string:X}Mixing records with maps-of-string-keys can be a bit of an antipattern: for example, in your code above, what happens if your package map needs to store an item that happens to be called "Settings"? I've seen situations like these where people say "I'll never have a conflict"... until they do. And that could lead to hard-to-track bugs, because there would be no way to check at compile time that For that reason, if we were to have maprecords, I'd be tempted to support it for any key type but string at first (one could use enums instead of strings, if they know the finite universe of valid keys beforehand). At least the 2. Arrayrecords should really be an intersection typeArrayrecords were a very early addition to the language, and since we added union types it would be more consistent to support intersection types instead. Not just for arrayrecords and maprecords, but for all table types that could be safely intersected. For example: local type MyArrayRecord = {string} & MyRecord
local type MyTupleRecord = {number, number, number} & MyRecord
local type MyArrayMap = {number} & {string:boolean}
local type MyMapMap = {string:number} & {number:string}
-- ...and so on!
-- as long as the keys don't conflict, we could intersect table types Union types currently have a cautious restriction in which you can only add one table type per union (to avoid runtime issues when discriminating the concrete type of the value, which is done using Lua's |
arrayrecords stood out to me as weird and inconsistent as well. I guess I just went on the opposite route to making them consistent, I also had an enumrecord proposal in mind. I agree that intersection types would be significantly better though. With your change, the Btw, since you've mentioned unions. Teal really should allow arbitrary unions, even if the |
I like this idea a lot. I've tried to type a lot of existing Lua code and I think this would make that easier. I would especially like an integer key type (as local record R
[1]: boolean
[2]: string
field: string
end |
It would be more consistent to allow records to also be maps, not just arrays.
It's a common Lua pattern to have a map which also has added (meta) fields on top.
In particular, something like this is very handy: a map, where some values are of known type, but it could have other arbitrary values too
Currently the best alternative I can think of is adding
metamethod __index: function(self: Package, key: string): any
.That's not terrible, but doesn't support the
.
syntax (yet?), only the square brackets.The text was updated successfully, but these errors were encountered: