elm / compiler

Compiler for Elm, a functional language for reliable webapps.
https://elm-lang.org/
BSD 3-Clause "New" or "Revised" License
7.49k stars 659 forks source link

Proposal: remove record syntax for field addition and field deletion #985

Closed evancz closed 8 years ago

evancz commented 9 years ago

The idea is to remove the following syntax:

{ point - z }      -- remove field z

{ point | z = 4 }  -- add field z

We keep all the type system stuff that makes record update work, but this surface syntax is removed.

I created the simple-records branch which actually implements this.

Talking with @JoeyEremondi, it came up that these operations may impose a significant performance cost. I had a discussion along these lines with one of the creators of Julia at Curry On which kind of pushed me over on this.

Arguments in favor of removal

Usage: I have never seen either of these features used in practice. Furthermore, the uses that I have seen are crazy and cool (see @Apanatshka's example here) but in a way that I'd probably recommend against in general :)

I also suspect there's something antimodular about this in that you can add a field x to any record, even one that already has an x. This issue has never come up in practice (since no one uses this) but it's something to think about.

Performance: The fact that you can add and remove fields in JS was a big part of why performance was terrible back before Chrome came along and started mapping things onto "static classes" as much as possible (i.e. fixed set of fields so that lookups are constant time)

We piggy back on their work for this, but this will not be true if we are compiling to assembly some day. The ideal case is that we know the exact shape of a record, so we'd have the following code gen:

-- elm
point = { x = 0, y = 0, z = 0 }
newPoint = { point | x <- 4 }
// generated js
var point = { x: 0, y: 0, z: 0 };
var newPoint = { x: 4, y: point.y, z: point.z }

This may mean we can just copy the bits when we are going lower level. Furthermore, when you are doing field lookup, the fastest way is if you know the offset of each field so that lookup time is constant: r.x becomes "get offset N in r" and I think this will be harder if we have extension. We'll at least have to do a bunch of work, and it's all for a feature that seems not too useful so far.

Remaining Questions

Okay, this is a distraction. It does not change the core idea at all, but it's the part of this proposal that is the most controversial.

Now that = is free, maybe it makes sense to use that for record updates. In other words, should we switch from <- to =?

Let's talk about this only to the extent that it touches on the broader question.

TheSeamau5 commented 9 years ago

Here's todo-mvc with equal signs instead of arrows. I personally prefer equal signs to arrows. https://gist.github.com/TheSeamau5/f08d4e5e6bf0967a91a6

evancz commented 9 years ago

Thanks @TheSeamau5 :) I find that case + record updates often means a mix of -> and <- that can look quite odd.

TheSeamau5 commented 9 years ago

Yup, I agree. Especially given the fact that record updates happen very often within case expressions.

mgold commented 9 years ago

To play devil's advocate, this feature gives you stacks that are checked for underflow at compile time:

push x s = {s| data = x}
peek s = s.data
pop s = {s - data}
empty = {}

--legal
val1 = empty |> push 3 |> push 7 |> peek
val2 = empty |> push 3 |> push 7 |> pop |> peek

-- compile-time error!
val3 = empty |> push 3 |> push 7 |> pop |> pop |> peek

But then I tried myStack = List.foldl push empty [1..4] and caused the compiler to stack overflow. So yeah, get rid of it. And switch to = too.

evancz commented 9 years ago

With the stack thing that Jeff noticed, I think you can do the same with ADTs. It's the same type trick either way.

type EmptyStack = EmptyStack
type StackFrame a stack = StackFrame a stack

push x stack = StackFrame x stack
peek (StackFrame x _) = x
pop (StackFrame _ stack) = stack
...

You don't get the kind protection where the substacks must be StackFrame or EmptyStack, but generally speaking, you can still do this.

In any case, I wouldn't say this example should guide our overall thinking. Can't imagine typed stacks being a major feature.

rtfeldman commented 9 years ago

The reasons for removing add and delete make sense to me, and I've never used either feature, so +1 from me.

On the separate syntax question, I have no strong feelings. Either <- or = seem fine.

Apanatshka commented 9 years ago

LGTM. Don't care about <- vs. =, I'd rather get rid of that whole update syntax and use something more functional. But that's a discussion which is already going on in another proposal ;)

evancz commented 9 years ago

One thing that is not in that discussion is a concern about performance. If you want to update 3 fields, record update syntax is going to be way faster. The compiler maybe can be clever, but this way it's not necessary. The other issue should have that as part of the discussion if it is going to be an alternative.

texastoland commented 9 years ago

I mentioned that to Richard this morning. IIUC as long as the function is fully applied then the compiler should be able to emit the same syntax as currently. When it's curried all bets are off.

rtfeldman commented 9 years ago

@evancz We chatted about it briefly at the end of this comment and the two after it, but did not come up with any solutions.

I proposed a pipe-dream-ish "best of all worlds" solution just now, but I suspect it may be impossible.

rtfeldman commented 9 years ago

+1 for = over <-

I've actually tried to use that syntax instinctively, and filed an error message catalog issue that could be closed if that change were accepted: https://github.com/evancz/error-message-catalog/issues/16

Also, it's objectively more concise. :wink:

evancz commented 9 years ago

Note: this comment has been edited heavily!

This forces us to 0.16 which seems okay, but I'm not 100% sure what else is going to happen. For example, I got rid of cascading errors the other day and am trying to bring in the incomplete pattern match warnings. Both are patch changes. Not sure how to time all of this.

I guess earlier is better on a change like this one, and we have a real story to tell about "regular and diffable syntax" if we get rid of multi-way if as well (and do string interpolation) (and do the trailing comma thing like JS and Python and others). Maybe it makes sense to batch all this so we can tell a coherent story.

mgold commented 9 years ago

My two cents: keep the breaking changes in branches, get the patch-level features done, and then decide if 0.16 looks far away enough to make 0.15.2 worthwhile.

Since when are we getting rid of multi-way if? I don't like the pipes but the language construct is useful. String interpolation would be great. There was a thread on the list about redoing mailboxes and I think we should try to fit that into 0.16 as well. Also, opinions are divided on the comma proposal.

texastoland commented 9 years ago

@mgold We're working on migrating these discussions to elm-plans for more focused discussion and visibility into process. Please raise issues there. It'll be announced on the mailing list soon along with regular live remote meetings. Looking forward to collaborating!

mgold commented 9 years ago

Got it, thanks, I'll comment on the individual concerns in their respective threads.

ThomasWeiser commented 9 years ago

I actually use field addition and deletion to check certain constraints in a DSL at compile time.

I am not sure if it's worth the hassle and I am not advocating for or against the proposal. I just want to illustrate that there are use cases for field additions/removals, which facilitate compile time checks that would be hardly possible otherwise.

In my Firebase wrapper ElmFire there are functions to specify a query like that:

q1 = valueChanged |> orderByChild "height" |> limitToLast 2
q2 = childAdded |> orderByPriority |> startAtPriority (NumberPriority 5) Nothing

There are a bunch of constraints how these options may be combined. For example you can use ony one orderByX option and you can use startAtPriority only when used together with orderByPriority.

Using record fields to sum up the options used so far enables us to check the validity of the next option.

These a the relevant types and functions used in the example above:

{-| A query specification: event type, ordering, filtering, limiting -}
type alias Query q = { q | tag : QueryOptions }

type QueryOptions = QueryOptions
emptyOptions =
  { tag = QueryOptions
  , noOrder = True
  , noLimit = True
  , noStart = True
  , noEnd = True }

type QueryEvent =
  ValueChanged | ChildAdded | ChildChanged | ChildRemoved | ChildMoved

valueChanged =
  { emptyOptions | queryEvent = ValueChanged }

orderByChild :    String
               -> { r | noOrder : a }
               -> { r | orderByChildOrValue : Maybe String }
orderByChild key query =
  { query - noOrder | orderByChildOrValue = Just key }

orderByPriority : { r | noOrder : a }
               -> { r | orderByPriority : Bool }
orderByPriority query =
  { query - noOrder | orderByPriority = True }

startAtValue :    JE.Value
               -> { r | noStart : a, orderByChildOrValue : o }
               -> { r | orderByChildOrValue : o, startAtValue: JE.Value }
startAtValue value query =
  { query - noStart | startAtValue = value }

startAtPriority : Priority -> Maybe String
               -> { r | noStart : a, orderByPriority : o }
               -> { r | orderByPriority : o, startAtPriority: (Priority, Maybe String) }
startAtPriority priority key query =
  { query - noStart | startAtPriority = (priority, key) }

limitToFirst :    Int
               -> { r | noLimit : a }
               -> { r | limitToFirst : Int }
limitToFirst num query =
  { query - noLimit | limitToFirst = num }

limitToLast :     Int
               -> { r | noLimit : a }
               -> { r | limitToLast : Int }
limitToLast num query =
  { query - noLimit | limitToLast = num }

Complete code for query specs is here.

Apanatshka commented 9 years ago

I think this is a good example of something useful you can do with addition/deletion. If at all possible, I'd like addition and deletion of fields to stay around in a way that's hard to stumble upon accidentally, more as an advanced feature that you can use for things like this.

Dobiasd commented 9 years ago

I'm in favor of this proposal. I never needed "remove" or "add" for records. And it seems like right now this even can cause inconsistent compiler behavior.

texastoland commented 9 years ago

I actually used it last night making an example to experiment with components. I'd like to wait till architecture questions are resolved to evaluate impact on major demos.

jessitron commented 9 years ago

+1, because I tried to use = to update a value in a record. <- did not occur to me. = is the original way to put values in a record (at initialization), and I'd love to use it for copy-constructor-style changes too.

As to adding or removing a field from a record, haven't wanted to. If I did do it, because it would be rare, I'd rather have a named function than syntax.

evancz commented 9 years ago

Okay, thanks for all the feedback! :)

I talked with a JS dev new to Elm at the meetup last week and he was running into lots of -> and <- in ways that felt weird. So between seeing that and the feedback here, I'm pretty into this idea.

The remaining concerns for me are:

evancz commented 8 years ago

Alright, I am closing in favor of https://github.com/elm-lang/elm-plans/issues/16 which will track a couple things along these lines. It's good to have them all in one place. Furthermore, I think it's not making sense to have proposals in the elm-compiler repo anymore, so I am laying the groundwork for doing this in a better way. For now, please let me manage elm-plans, no new stuff there!

We should continue the discussion of this idea in this issue though!

ThomasWeiser commented 8 years ago

My use case mentioned above (constraints in a DSL in the API of ElmFire) should not block this proposal in any way.

I can surely drop the compile-time checks and instead check the constraints at run-time. The tasks involved can fail anyway, so this would be just another error case.

In general, checking constraints on combining pieces of data at compile-time is nice. But I guess such combinations will be a static thing in most applications, so violating them would be obvious quickly with basic testing.

Dobiasd commented 8 years ago

The case of @ThomasWeiser kind of convinced me, that addition and deletion can be very useful after all. I guess I initially only voted in favour of the proposal because of the assosiated bugs (1,2). If they will be fixed fixed, I would vote for keeping record addition and deletion in the language.

rtfeldman commented 8 years ago

If I'm understanding correctly, the motivation behind this DSL is to avoid bugs where you accidentally specify something twice, and only meant to specify it once. If all the DSL can do is update syntax, then you can accidentally call startAtValue twice on the same query, and the second one will "win" - whereas it would be better to make that a compile error, so you could make sure only to call it once.

Assuming that's the case, couldn't you get the same level of safety if this change were coupled with a "no duplicate fields in the same record, and also no duplicate fields in record updates" rule? (In other words, { foo | bar <- 3, bar <- 4 } is always a compile error; you can't repeat fields in the same update.)

In that case you wouldn't even need a DSL here; compare:

-- Current
emptyOptions = { noOrder = True, noLimit = True }

q1 = valueChanged |> orderByChild "height" |> limitToLast 2
q2 = childAdded |> orderByPriority |> startAtPriority (NumberPriority 5) Nothing
-- Alternative
type OrderOptions = NoOrder | ByPriority | ByChild String
type LimitOptions = NoLimit | ToFirst Int | ToLast Int
type StartOptions = NoStart | StartAtValue JE.Value | StartAtPriority Priority (Maybe String)

defaults = { order = NoOrder, limit = NoLimit, start = NoStart }

q1 = { defaults | queryEvent = ValueChanged, order = ByChild "height, limit = ToLast 2 }
-- Formerly: q1 = valueChanged |> orderByChild "height" |> limitToLast 2

q2 = { defaults | queryEvent = ChildAdded, order = ByPriority, start = StartAtPriority (NumberPriority 5) Nothing }
-- Formerly: q2 = childAdded |> orderByPriority |> startAtPriority (NumberPriority 5) Nothing

Assuming there's a "no duplicate fields in records or record updates" rule in effect, you now have the same characteristics as before:

Thoughts?

ThomasWeiser commented 8 years ago

If I'm understanding correctly, the motivation behind this DSL is to avoid bugs where you accidentally specify something twice, and only meant to specify it once.

Thanks Richard, this is one type of constraints, and I agree this could be expressed the way you propose (given the new "no duplicate fields in records or record updates" rule).

There is an additional constraint that I currently check, e.g.: startAtPriority is only valid if it's used with orderByPriority, startAtValue is only valid if used with orderByChild or orderByValue, etc. In other words: the parameter type of startAt and endAt depends on the used ordering.

I couldn't find a way to check at compile-time without using record-field addition/deletion.

ThomasWeiser commented 8 years ago

I have a feeling that some other use cases of field addition/deletion may emerge in the future. So I would propose to remove the syntax only but leave the operations in place (using named functions).

I'm not sure I fully understand Evan's original performance concerns. I don't think that dynamically updating existing JS objects is involved here.

But I would appreciate the proposed syntax change (= for updating a field). And it should be possible to update several fields at once (both for nicer code and for performance reasons):

m1 = { x = 1, y = 2, z = 3 }
m2 = { m1 | x = 10, y = 20 }
rtfeldman commented 8 years ago

startAtPriority is only valid if it's used with orderByPriority, startAtValue is only valid if used with orderByChild or orderByValue, etc. In other words: the parameter type of startAt and endAt depends on the used ordering...I couldn't find a way to check at compile-time without using record-field addition/deletion.

Ah! I'd encode that restriction into the union type, e.g.

type OrderOptions
  = NoOrder 
  | ByPriority (Maybe (Priority (Maybe String))) -- Nothing means no startAtPriority
  | ByChild String (Maybe JE.Value) -- Nothing means no startAtValue

type LimitOptions = NoLimit | ToFirst Int | ToLast Int

-- This is now obsolete
-- type StartOptions = NoStart | StartAtValue JE.Value | StartAtPriority Priority (Maybe String)
mgold commented 8 years ago

Yup, using union types to make impossible states unrepresentable is a great skill to have.

ThomasWeiser commented 8 years ago

Yep, that should work. Thanks for your efforts, much appreciated!

rtfeldman commented 8 years ago

Happy to help! :smile_cat:

Dobiasd commented 8 years ago

Please excuse my inanity, @rtfeldman, but I do not yet understand how the union approach can prevent mistakes like this

unorderedThing |> startAtPriority

at compile time as opposed to valid operations like that

unorderedThing |> orderByPriority |> startAtPriority

Would you mind elaborating or giving a minimal example?

evancz commented 8 years ago

I'd probably approach the API by trying to only allow the construction of acceptable values. So maybe have a few ways to construct a Query where you do all the settings as arguments, not as functions that modify a Query. I think with Richard's route about favoring union types it may even be possible to have the functions and be sure that you always end up with a Query in a safe configuration.

As things stand, I tend to see this as a recommendation for removing the syntax for extension and deletion. Perhaps the realization here is that "it's possible to encode these problems with union types and end up with a simpler API" which is actually a pretty cool insight if I'm understanding things correctly.

texastoland commented 8 years ago

Perhaps the realization here is that "it's possible to encode these problems with union types and end up with a simpler API" which is actually a pretty cool insight if I'm understanding things correctly.

Is that the same as saying extension and deletion is a nice sugar for problems that need union types in other MLs though? Maybe I'm having trouble envisioning the difference. I could post the code I used this feature for but I've been waiting for a better pattern for shared component state.

rtfeldman commented 8 years ago

I don't think so; as per the original post, they're more than just "nice sugar" - they have some legit drawbacks!

rgrempel commented 8 years ago

Once this is done (in fact, I think it has been, since the simpler-records branch has now been merged), is code of the following kind still intended to work? (Note that I would just build the master branch and see, but I'm having trouble doing that right now, and I think it's a known problem at the moment that's beyond my ability to solve.)

Note that I've simplified this a bit, and it's incomplete (just imagine the extra bits that would make this compile) -- so the point of this is modular construction of an extensible record.

The reason I ask is that what initialLanguageModel, initialAccountModel and initialFocusModel are doing below amounts to adding a new field -- that is, they take a record of one type, and return a record of a different type by adding a field. However, they aren't literally using the syntax that has been removed. So perhaps this is still intended to work?

My apologies if it was obvious that this was still intended to work -- I may be worrying about nothing.

-- in Main.elm, with suitable imports
type alias Model =
    LanguageModel (
    AccountModel (
    FocusModel (
      {}
    )))

initialModel : Model
initialModel =
    initialAccountModel <|
    initialLanguageModel <|
    initialFocusModel <|
    {}

-- in Language.elm
type alias LanguageModel m =
    { m | useLanguage : Language }

initialLanguageModel : m -> LanguageModel m
initialLanguageModel model = LanguageModel EN model

-- in Account.elm
type alias AccountModel m =
    { m | currentUser : Maybe User }

initialAccountModel : m -> AccountModel m
initialAccountModel model = AccountModel Nothing model

-- in Focus.elm
type alias FocusModel m =
    { m | focus : Focus }

initialFocusModel : m -> FocusModel m
initialFocusModel model = FocusModel (Home HomeTypes.Home) model
mgold commented 8 years ago

I have a similar question, will this still work?

distFromOrigin : {a| x : Float, y : Float} -> Float
distFromOrigin {x, y} = sqrt <| x*x + y*y

foo = distFromOrigin {x = 5, y = 2, dx = 3, dy = -9.8}
bar = distFromOrigin {x = 1, y = 4}

This is the closest thing Elm has to interfaces and related features. It's true that you can't keep these records in a list (they have different types) but being able to apply the same function to both is very handy.

evancz commented 8 years ago

@rgrempel, your thing will not work. Even though it is different source syntax, it is defined in terms of the underlying record extension mechanism. So in the master branch, you can create record constructors for records with known shape, but cannot make record constructors that add a bunch of fields.

@mgold, your stuff all works. None of the types are changing, there is just a few syntactic constructs that are going away.

rgrempel commented 8 years ago

Thanks. I guess I have a follow-up question. Isn't what I was doing part of Elm's ambition for extensible records?

I'm thinking of this page:

http://elm-lang.org/docs/records

... and in particular, everything following "You can also define extensible records. This is generally recommended because it makes your functions more reusable."

I guess what confuses me a little is that the discussion above was partly premised on the idea that no one uses this stuff -- e.g. "I have never seen either of these features used in practice. Furthermore, the uses that I have seen are crazy and cool (see @Apanatshka's example here) but in a way that I'd probably recommend against in general :)"

But, I thought that the idea of extensible records was actually pretty important to Elm, and pretty useful.

Now, perhaps it's the case that we'll still be able to define records extensibly -- that is, perhaps this syntax (from the Elm documentation page mentioned above) is still intended to work.

type alias Positioned a =
  { a |
      x : Float,
      y : Float
  }

type alias Named a =
  { a |
      name : String
  }

type alias Moving a =
  { a |
      velocity : Float,
      angle : Float
  }

dude : Named (Moving (Positioned {}))
dude =
  { x = 0
  , y = 0
  , name = "Clark Kent"
  , velocity = 42
  , angle = degrees 30
  }

But, if I understand you correctly, this is only intended to work if dude is fully initialized all-at-once ... that is, the record is extensible in terms of the type system, but not extensible at run-time. That seems like a strange gap -- not a "no one uses this exotic feature" type of thing.

For instance, it would mean that while the type definition can be modularized -- that is, the various parts of the extensible record can be defined in different modules for the purposes of the type system -- but, the actual initialization of the data cannot be modularized -- it would have to happen all-at-once at the top level. It seems to me that this will be inconvenient for programming "in the large" -- since Elm promotes the idea of a single, master model type, for instance, to which many, many modules might contribute in a large program. You would end up with a ton of decisions about initial state being located in a central module, when it would be much more convenient to locate the initial state with the definition of the types in the various sub-modules.

Now, clearly the central module is going to have to reference them. But, at least modification could then happen purely in the lower-level module. Imagine, for instance, if you have 20 submodules that define aspects of your big Model type. I don't see that as a ridiculous example at all -- Elm will get used in apps that big, and bigger, eventually, if not already. Now, imagine changing the type alias in one of the submodules to add a new field. With record extension at run-time, you can change the initial value of the data right in its own submodule (since it already is referenced from the main module). Without record extension at run-time, you have to change the initial value in the main module.

All of which to say that I think something important has been removed here, not something exotic. (It turns out that a reasonable degree of modularity is still possible -- see comments below)

rgrempel commented 8 years ago

Having thought this through a little more, I have realized that one can still achieve a reasonable level of modularity after this change.

What one would need to do is something like this:

That is, even if the field names can no longer be added by sub-modules, the right-hand side -- that is, the values -- can still be supplied by referring to sub-modules.

To make this concrete, I think my original example could be re-written something like this:

-- in Main.elm, with suitable imports
type alias Model =
    { language : LanguageModel
    , account : AccountModel
    , focus : FocusModel
    }

initialModel : Model
initialModel =
    { language = initialLanguageModel 
    , account = initialAccountModel
    , focus = initialFocusModel
    }

-- in Language.elm
type alias LanguageModel =
    {  useLanguage : Language }

initialLanguageModel : LanguageModel
initialLanguageModel = LanguageModel EN

-- in Account.elm
type alias AccountModel =
    { currentUser : Maybe User }

initialAccountModel : AccountModel
initialAccountModel = AccountModel Nothing

-- in Focus.elm
type alias FocusModel =
    { focus : Focus }

initialFocusModel : FocusModel
initialFocusModel = FocusModel (Home HomeTypes.Home)

Then, imagine adding an additional field to LanguageModel, AccountModel or FocusModel. You'd only need to modify the sub-module -- you could add the new field to the type, and to the initial value, and you wouldn't need to touch the main module.

In fact, philosophically, it might make some sense to have to list the field names in the main module explicitly -- after all, in a way it makes sense for the main module to be in charge of the field names, since it is with respect to the overall record type that those names should not clash (or, at least, it is the main module that ought to be explicitly aware of such clashes). So, this is sort of like the way that the main module is in charge of the tag names for union types, even where the various types it is unifying are themselves defined in separate sub-modules.

Now, compared to my previous code, there would be an extra layer of indirection -- that is, what one could previously refer to as model.useLanguage would now be model.language.useLanguage. But, I'm not sure that's such a bad idea -- it leaves the 'tagging' (so to speak) of the main Model to the main module, which might be sensible -- again, it's sort of like the way union type tags work.

So, I withdraw my previous comment. Well, I suppose I could have just deleted it, but I'll leave it and this in case anyone finds the topic interesting.

dobesv commented 8 years ago

In general there are a lot of features we can live without. We can theoretically just program in lambda calculus, or assembly language. So, it's not necessarily a question of where a feature is a necessity, especially in this case. Rather, it's a cost benefit analysis.

I definitely have mixed feelings about this proposal. I like simplification, that reduces the cost. And it sounds like the benefits of this particular feature turned out to be small so far. But I also thought the row polymorphism stuff was pretty sexy, including adding fields. It can be hard to let go :).

I think there is potential in record concatenation instead of fields addition. But that's a whole other story and I don't have much to back that up yet. It would have a different syntax and it should replace field addition since you can append whatever fields you were going to add. So, it doesn't go counter to this proposal either.

So based on the limited data I have it seems okay to let go of field add and delete syntax.

rgrempel commented 8 years ago

I updated my actual code so it will work without adding fields at run-time. There was one little twist I didn't anticipate, but it wasn't that bad -- the results are pretty reasonable. Here's the commit, in case anyone's interested in what difference it made:

https://github.com/rgrempel/csrs-elm/commit/49b9b165c06d24b9deb009fcaa4ea0b0e967b405

laszlopandy commented 8 years ago

Wow this is a great example of using records. I think doing it the new way makes reading your core much clearer.

On Saturday, September 5, 2015, Ryan Rempel notifications@github.com wrote:

I updated my actual code so it will work without adding fields at run-time. There was one little twist I didn't anticipate, but it wasn't that bad -- the results are pretty reasonable. Here's the commit, in case anyone's interested in what difference it made:

rgrempel/csrs-elm@49b9b16 https://github.com/rgrempel/csrs-elm/commit/49b9b165c06d24b9deb009fcaa4ea0b0e967b405

— Reply to this email directly or view it on GitHub https://github.com/elm-lang/elm-compiler/issues/985#issuecomment-137925032 .

Warry commented 8 years ago

It's not entirely related, but while playing with Focus-like kind of manipulations, I thought that if the .prop to "get" a property became something more like the :keyword in clojurescript along with getter and setter functions, it would allow a big deal of abstraction on top of records:

getFoo = get .foo
setFoo = set .foo

mario =
    { physics = { position = { x=3, y=4 }
                , velocity = { x=1, y=1 }
                }
    }

freeze object =
    Focus.set (.physics => .velocity) { x=0, y=0 } object

Do you think that it's interesting enough to start a conversation over the forum or another issue?

laszlopandy commented 8 years ago

@Warry the discussion for that is over here: https://github.com/elm-lang/elm-compiler/issues/984

Warry commented 8 years ago

thanks @laszlopandy Looks like I constantly arrive after the battle ^^ I must be 2 months late regarding the rest of the community :->

jon49 commented 8 years ago

Not sure if this would be appropriate here. But I'm just starting to learn Elm. And I thought the syntax was pretty smart with the delineation with = and <-, coming from an F# background (although I would not consider myself proficient in said language).

I do find that I can do the following a bit disconcerting though:

type alias Person = { name : String, age : Int }

bill : Person
bill = { name = "Gates", age = 57 }

suzy = { bill | name <- 7 }

The behavior I expected was that suzy would still be a Person and that a compile error would be thrown when setting name in Person as 7. I saw the = and thought that was a nice use case for changing types:

-- OK since replacing the type from { name : String } to { name : Int }
-- i.e., creating a new type
jill = { bill | name = 7 }

-- expect compile error since just updating the record
-- I would expect the inferred type would remain `Person`
suzy = { bill | name <- 7 }
Deimos28 commented 7 years ago

I realize I'm a tad late on this - in fact I'm writing the following as much for personal reference as for helping others who may run into the same problem as me.

Elm requires having a top-level "Model" type that contains everything. Note that "contains everything" does not necessarily imply "knows the details of everything". When building an app using generic widgets (eg. a date-time picker), we generally do not want the top-level Model (or the other widgets, for that matter) to know the knitty-gritty of our date-time picker. So you use an opaque type for the date-time picker and put it as a field in the top-level record type Model. That's fine, but often you have one or more layers of widgets between the more generic (ie. reusable) low-level widget (such as the date-time picker) and the completely specific overall application and that's where the problems start.

In Elm's architecture tutorial, there are two extreme cases:

However, I find myself in a situation where I have three layers:

Part of my model must be shared by all subwidgets, while other parts (eg. the elementary widgets' model) are completely specific. I haven't been able to find any examples of this (certainly not in the elm architecture tutorial). I discovered I came up with the same pattern as @rgrempel when initializing my widgets:

At the top-level (Widget0):

type alias Model a =
  { a
  | foo : Int
  | widget1 : Widget1.InternalModel
  | widget2 : Widget2.InternalModel

In module Widget1:

type alias Model a =
  { a
  | foo : Int
  , widget1 : InternalModel
  }

type InternalModel = Foo | Bar | Baz | Whatever

In module Widget2:

type alias Model a =
  { a
  | foo : Int
  , widget2 : InternalModel
  }

type InternalModel = {bla : Int, azd : String }

Just like @rgrempel, I want each module also provides its 'init' function to initialize its part of Widget0's model. With extensible records, I could simply do (eg. in Widget1):

init : a -> Model a
init foo =
  {foo | widget1 = initInternal}

and for Widget0 I would have the very clean and composible chain:

init : Model
init =
  {foo : 345}
  |> Widget1.init
  |> Widget2.init

So the extensible records were an easy way for me to build composable applications. With the removal of this feature, the initfunction can no longer be type-parametrized. This is really important so let me emphasize a bit: no extensible records means init must know the full record it operates on.

So I decoded to use @rgrempel's strategy, but he was in a case where all parts of his model were independent. The only workaround I found is to do the following for Wiget0 (top-level):

type alias Model a =
  { a
  | common : {foo : Int}
  , widget1 : Widget1.InternalModel
  , widget2 : Widget2.InternalModel
  }

and then, eg. for widget 2:

type alias Model a =
  { a
  | common : {foo : Int}
  , widget2 : InternalModel
  }

It's probably cleaner to define "common" in its own module (that way the submodules don't need to know about the top-level module)

type alias Common =
  { foo : Int }

init : Common
init = { foo = 321 }

Then, at the top-level Widget0 you have:

type alias Model =
  { common : Common
  , widget1 : Widget1.InternalModel
  , widget2 : Widget2.InternalModel
  }

init : Model
init =
  { common = Common.init
  , widget1 = Widget1.init
  , widget2 = Widget2.init
  }

The important thing to note here is that you must separate the internal part of the model from the common part: the full models of Widget1 and Widget2 are not exported - just their internal part (which seems a bit paradoxical, as the internal parts are typically the ones you wouldn't want to expose).

That in itself is not a problem. It becomes one when you loose extensible records because you have to put all common parts in a Common record, which essentially means you know in advance how your widget will be used. For instance, what do you do if the data shared by Widget1 and Widget2 is not the same as the data shared by Widget2 and Widget3? Do you have two commons? Do you have one big common (and all widgets have to know about stuff they'll never need)? What happens when the application grows and a new widget Widget4 needs to share some of its data with Widget1 and Widget3?

As previously stated, as soon as you define an init function, the record it returns (or operates on) can no longer be type-parametrized (i.e. extensible), which means that if init returns the Common part, Common is no longer extensible. If Common is not extensible & you want to include Widget0 in a bigger application, Widget0's model will have to be completely independent from the other widgets' model at the same level, ie. it will not be able to share part of its model with the other widgets.

In fact, only the update & view functions can be type-parametrized to ensure the widgets don't have to know about one another:

update : Msg -> {a | common : Common, widget1 : Widget1.Internal} -> {a | common : Common, widget1 : Widget1.Internal}

With extensible records, you could apply the same pattern to any number of levels, but as soon as you put shared data in a common field you're basically preventing sharing something else (part of Common plus other fields) at the next level up the widget chain because Common can no longer be type-parametrized.

Sorry to ramble on about this, but it's been a thorn in my side for a long time now.

I would really appreciate any thoughts on this.

rtfeldman commented 7 years ago

@Deimos28 My initial thoughts:

  1. We try super hard to avoid the XY problem. So I would strongly recommend re-framing this in terms of the specific thing you're trying to build. "Widget1, Widget2, Widget3" doesn't give us an idea of the specific problem you actually ran into. I realize this requires giving extra context, but that context very often leads to a completely different answer. 😄
  2. The right place to post this is definitely elm-discuss.

See you over there!