Divide (closed) Primitives and (opened) Builtins #119

turboMaCk · 2021-09-24T18:49:52Z

Splitting ElmPrim into:

ElmPrim - language level constructs
ElmBuiltins - types defined by libraries (not generated by elm-street) like elm/core or elm/time etc.

ElmPrim keeps its original closed design using variants in sum type.

ElmBuiltins is using record/product to make implementation open. This gives elm-street several new features:

user definable Elm instances for types provided by library
ability to define Elm instance for parametrized type at least in manual fashion.

Full vocabulary of ElmDefinition:

DefPrim - primitive types defined by elm (language itself - not just by core or other lib)
DefBuiltin - ADTs not generated by elm-street (provided by libraries like core or time).
DefRecord - record types
DefType - ADTs generated by elm-street

Example definitions

-- simple type instance
instance Elm Value where
    toElmDefinition _ = DefBuiltin $ ElmBuiltin
        { builtinImplType = "Value"
        , builtinImplEncoder = "Basics.identity"
        , builtinImplDecoder = "D.value"
        , builtinImplParams = []
        }

-- parametrized type instance
instance (Elm a, Elm b) => Elm (Either a b) where
    toElmDefinition _ = DefBuiltin $ ElmBuiltin
        { builtinImplType = "Result"
        , builtinImplEncoder = "elmStreetEncodeEither"
        , builtinImplDecoder = "elmStreetDecodeEither"
        , builtinImplParams = [elmRef @a, elmRef @b]
        }

Future work to be done

printing functions do produce valid elm code by are naive (produce trailing white-spaces)
Elm.Generate module should be extended to make it simpler to include custom imports into generated files. (future PR)

Possibly further extensions:

support for phantom types (labeling params as regular or phatom params)
in case we would be able to derive this, this can completely replace ElmType giving it support for parametrized types.

Related issues:

this will become avoidable Generation of function names isn't namespace aware. #113
user can make own tradeoffs and implement Add Dict and Set primitive for Map, HashMap, IntMan, Set, HashSet and IntSet #26

related closed issues:

things like this would be in user's control Elm instance for NonEmpty #102
Define instance for of Elm for Data.Aeson.Value #111
Decide what to do with time data types? #27

@jhrcek This is not ready for merge but I figured it would be better to get your feedback as soon as I have working spike. Please feel free to comment. This PR resolves my single biggest frustration of working with elm-street.

@arbus I think you might also give input to this proposal

turboMaCk · 2021-09-24T18:59:27Z

frontend/src/Core/Decoder.elm

-    |> required "time" Iso.decoder
-    |> required "value" D.value
+    |> required "time" Iso.decoder 
+    |> required "value" D.value 


related to todo of making implementation less naive

turboMaCk · 2021-09-24T18:59:31Z

frontend/src/Core/Encoder.elm

-    , ("time", Iso.encode x.time)
-    , ("value", Basics.identity x.value)
+    , ("time", Iso.encode  x.time)
+    , ("value", Basics.identity  x.value)


related to todo of making implementation less naive

turboMaCk · 2021-09-24T19:00:29Z

frontend/src/Core/Types.elm

+type alias ElmStreetNonEmptyList a = (a, List a)
+


this is now required as we can't produce type annotation (a, List a) - but we can produce annotations using this alias.

turboMaCk · 2021-09-24T19:01:01Z

src/Elm/Ast.hs

-    | ElmTime                               -- ^ @Posix@ in elm, @UTCTime@ in Haskell
-    | ElmValue                              -- ^ @Json.Encode.Value@ in elm, @Data.Aeson.Value@ in Haskell
-    | ElmMaybe !TypeRef                     -- ^ @Maybe T@
-    | ElmResult !TypeRef !TypeRef           -- ^ @Result A B@ in elm
    | ElmPair !TypeRef !TypeRef             -- ^ @(A, B)@ in elm
    | ElmTriple !TypeRef !TypeRef !TypeRef  -- ^ @(A, B, C)@ in elm
-    | ElmList !TypeRef                      -- ^ @List A@ in elm
-    | ElmNonEmptyPair !TypeRef              -- ^ @NonEmpty A@ represented by @(A, List A)@ in elm
    deriving (Show)

+-- | Buitin types defined by core or 3rd party libraries
+-- Included definitions:
+-- * @Maybe a@
+-- * @Result a b@
+-- * @List a@
+-- * @Time.Posix@
+-- * @Json.Encode.Value@
+data ElmBuiltin = ElmBuiltin
+    { builtinImplType    :: !Text
+    , builtinImplEncoder :: !Text
+    , builtinImplDecoder :: !Text
+    , builtinImplParams  :: ![TypeRef]
+    } deriving (Show)
+


this is core of the PR

turboMaCk · 2021-09-24T19:01:37Z

src/Elm/Generic.hs

+instance Elm Value where
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "Value"
+        , builtinImplEncoder = "Basics.identity"
+        , builtinImplDecoder = "D.value"
+        , builtinImplParams = []
+        }
+
+instance Elm UTCTime where
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "Posix"
+        , builtinImplEncoder = "Iso.encode"
+        , builtinImplDecoder = "Iso.decoder"
+        , builtinImplParams = []
+        }

 instance Elm a => Elm (Maybe a) where
-    toElmDefinition _ = DefPrim $ ElmMaybe $ elmRef @a
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "Maybe"
+        , builtinImplEncoder = "elmStreetEncodeMaybe"
+        , builtinImplDecoder = "nullable"
+        , builtinImplParams = [elmRef @a]
+        }

 instance (Elm a, Elm b) => Elm (Either a b) where
-    toElmDefinition _ = DefPrim $ ElmResult (elmRef @a) (elmRef @b)
-
-instance (Elm a, Elm b) => Elm (a, b) where
-    toElmDefinition _ = DefPrim $ ElmPair (elmRef @a) (elmRef @b)
-
-instance (Elm a, Elm b, Elm c) => Elm (a, b, c) where
-    toElmDefinition _ = DefPrim $ ElmTriple (elmRef @a) (elmRef @b) (elmRef @c)
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "Result"
+        , builtinImplEncoder = "elmStreetEncodeEither"
+        , builtinImplDecoder = "elmStreetDecodeEither"
+        , builtinImplParams = [elmRef @a, elmRef @b]
+        }

 instance Elm a => Elm [a] where
-    toElmDefinition _ = DefPrim $ ElmList (elmRef @a)
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "List"
+        , builtinImplEncoder = "E.list"
+        , builtinImplDecoder = "D.list"
+        , builtinImplParams = [elmRef @a]
+        }

 instance Elm a => Elm (NonEmpty a) where
-    toElmDefinition _ = DefPrim $ ElmNonEmptyPair (elmRef @a)
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "ElmStreetNonEmptyList"
+        , builtinImplEncoder = "elmStreetEncodeNonEmpty"
+        , builtinImplDecoder = "elmStreetDecodeNonEmpty"
+        , builtinImplParams = [elmRef @a]
+        }


utilize new ElmBuiltin in instances we provide out of the box.

turboMaCk · 2021-09-24T19:33:48Z

frontend/src/Core/Types.elm

+    , time : Posix 
+    , value : Value 


related to todo of making implementation less naive

frontend/tests/Tests/Golden.elm

jhrcek

I can't point to anything that's obviously wrong.
In general it feels that adding more extensibility in this way could open a can of worms.
But this might be fine if we're the only users of the library (just checked that on hackage elm-street is pretty low in terms of number of downloads https://hackage.haskell.org/packages/search?terms=elm)

I expected that making the library extensible would be pretty hard, if for no other reason then just because you're trying to provide API for creating definitions in some other general-purpose programming language.

The piece that is most unclear to me is "how to add more custom definitions/pre-existing decoders etc. to the generated ElmStree.elm to support more user-defined types"

src/Elm/Ast.hs

jhrcek · 2021-09-27T09:34:51Z

src/Elm/Ast.hs

+-- * @Time.Posix@
+-- * @Json.Encode.Value@
+data ElmBuiltin = ElmBuiltin
+    { builtinImplType    :: !Text


Can we change the field prefixes to elmBuiltin? The word Impl doesn't look very informative..

Also I find the word "Builtin" a bit misleading.
It's feels to me to have the same meaning as "Elm primitive" as in "it's provided by Elm itself".
How about something like ElmLibrary / ElmLibType ?

Impl is leftover from development. I did the change incrementally by porting support from simpler to more complicated types to new structure it will need to be removed.

It's feels to me to have the same meaning as "Elm primitive" as in "it's provided by Elm itself".

agree though even this is already improvement over including these things into primitives. The one way in which these things are built in even if they come from 3rd party library (say time which depends on elm-iso8601-date-strings package) is that their decoders and definition is not generated by elm-street. Including Lib into the name is also option. Though this can be even defined in modules other than the ones generated by elm street as well. This might need some more thinking perhaps.

jhrcek · 2021-09-29T06:13:06Z

src/Elm/Ast.hs

+-- * @Json.Encode.Value@
+data ElmBuiltin = ElmBuiltin
+    { builtinImplType    :: !Text
+    , builtinImplEncoder :: !Text


As a user of this API It's not clear from the type what this Text is supposed to be.
Is it name of the function defined somewhere? (Where?)
Can I add my custom definitions in that place?
Is it supposed to be fully qualified?
Although it's clear from the usage examples, it might not be so clear to library users ..

this should be documented, hover before user will be able to define own things free-rely we need to make it possible to define custom imports for generated modules. So it doesn't make much sense documenting this before that part redesign so whole process could be documented.

jhrcek · 2021-09-29T06:14:17Z

src/Elm/Generic.hs

-    toElmDefinition _ = DefPrim $ ElmList (elmRef @a)
+    toElmDefinition _ = DefBuiltin $ ElmBuiltin
+        { builtinImplType = "List"
+        , builtinImplEncoder = "E.list"


The E. and D. in this definition seem magical. Where are they coming from?
EDIT: Ah, I see they're coming from

import Json.Decode as D exposing (..) import Json.Decode.Pipeline as D exposing (required) ... import Json.Encode as E exposing (..)

yes. I agreee this is not nice though. But I would probably rather address that in separate PR

Co-authored-by: Jan Hrcek <[email protected]>

turboMaCk added 6 commits September 24, 2021 18:27

Decouple Primitives and Builtins

25058e2

better organization

a5812ea

cosmetics

36ca36d

remove trailing whitespaces

3175db8

format

ef64980

spike open ElmBuiltin implementation

96228c8

turboMaCk marked this pull request as draft September 24, 2021 18:49

turboMaCk requested review from jhrcek and arbus September 24, 2021 18:50

turboMaCk changed the title ~~Divide (closed) Primitives and (opend) Builtins~~ Divide (closed) Primitives and (opened) Builtins Sep 24, 2021

turboMaCk commented Sep 24, 2021

View reviewed changes

frontend/tests/Tests/Golden.elm Outdated Show resolved Hide resolved

Update frontend/tests/Tests/Golden.elm

cbbe521

jhrcek reviewed Sep 29, 2021

View reviewed changes

Update src/Elm/Ast.hs

659bdb6

Co-authored-by: Jan Hrcek <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Divide (closed) Primitives and (opened) Builtins #119

Divide (closed) Primitives and (opened) Builtins #119

turboMaCk commented Sep 24, 2021 •

edited

Loading

turboMaCk Sep 24, 2021

turboMaCk Sep 24, 2021

turboMaCk Sep 24, 2021 •

edited

Loading

turboMaCk Sep 24, 2021

turboMaCk Sep 24, 2021

turboMaCk Sep 24, 2021

jhrcek left a comment

jhrcek Sep 27, 2021

jhrcek Sep 29, 2021 •

edited

Loading

turboMaCk Oct 5, 2021

jhrcek Sep 29, 2021

turboMaCk Sep 29, 2021

jhrcek Sep 29, 2021 •

edited

Loading

turboMaCk Sep 29, 2021

Divide (closed) Primitives and (opened) Builtins #119

Are you sure you want to change the base?

Divide (closed) Primitives and (opened) Builtins #119

Conversation

turboMaCk commented Sep 24, 2021 • edited Loading

Example definitions

Future work to be done

Possibly further extensions:

Related issues:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turboMaCk Sep 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhrcek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhrcek Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhrcek Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turboMaCk commented Sep 24, 2021 •

edited

Loading

turboMaCk Sep 24, 2021 •

edited

Loading

jhrcek Sep 29, 2021 •

edited

Loading

jhrcek Sep 29, 2021 •

edited

Loading