ChristophP / elm-i18next

https://package.elm-lang.org/packages/ChristophP/elm-i18next/latest
BSD 3-Clause "New" or "Revised" License
67 stars 13 forks source link

Expose Translations constructor #20

Closed Tehnix closed 4 years ago

Tehnix commented 4 years ago

To be able to create your own translations (e.g. hardcoded) inside Elm, it is necessary to expose the Translations constructor, to make this possible.

My specific use case is to make it easy to switch between dynamic translations, fetched via HTTP, and hardcoded translations, since the feasibility of each can vary with project size.

This makes something like the following possible:

langEn : Translations
langEn =
    let
        text =
            [ ( "sharedTextToReverse", "Text To Reverse" )
            , ( "sharedReverseText", "Reverse Text" )
            , ( "titleItem", "Item!" )
            , ( "titleNotFound", "Not Found!" )
            ]
    in
    Translations <|
        Dict.fromList
            text
ChristophP commented 4 years ago

Hi, thanks for opening a PR. I see you want to expose the Translations constructor. Unfortunately that isn't something I wanna do because I want to keep the type opaque so that internal changes won't be breaking for users and to comply with the elm package design guidelines. However, I am willing to consider your use case and add a function that basically aliases the constructor to let you do what you need to do. Could you explain your use case in a bit more detail please?

My specific use case is to make it easy to switch between dynamic translations, fetched via HTTP, and hardcoded translations, since the feasibility of each can vary with project size.

Why do you want to have translations hardcoded? Would hardcoding them as JSON and simply decoding it work too?

Tehnix commented 4 years ago

Ah, that makes sense that you don't wanna expose the internal.

What I'm essentially after is a way to construct the Translations, where my gut reaction was just to expose the constructor, since it's a simple Dict underneath. I use this so that I can allow the user that is setting up a project to switch between dynamic (e.g. via locize) or hardcoded translations easily, while being able to reuse the t/tr etc functions that are already built.


Right now it's possible to use the JSON decoder, but that throws away the type safety of constructing the translations. E.g. you have,

import I18Next
import Json.Decode as Decode

exampleTranslations : I18Next.Translations
exampleTranslations =
    case Decode.decodeString I18Next.translationsDecoder "{ \"greet\": \"Hello\" }" of
        Ok t -> t
        Err _ -> I18Next.initialTranslations

Where you can make the "{ \"greet\": \"Hello\" }" a bit more manageable to work with, but you still end up with working with a string, and loosing type safety, and the burden of dealing with JSON decoding.

A better solution is maybe to expose a I18Next.fromList (or similar), which turns [ ("key", "value") ] into the Translations type. This would allow you to still keep the internals hidden, while providing an API that is quite familiar.

As a bonus, the implementation of I18Next.fromList is just handed off to Dict, so long as the internal structure is also a Dict.

I don't know if that made sense?

ChristophP commented 4 years ago

@Tehnix Gotcha, I see the problems that string decoding gives you. Your suggestion about fromList that's very close to what I had in mind. But there's one thing that I found problematic, it doesn't allow for any nesting and makes you repeat all key prefixes everywhere. Your example included a very simple set of keys without any nesting but a production hardcoded translation list could look like this:

[ ("common.greeting.morning", "Morning") -- lots of key repitition here, increased risk for typos
, ("common.greeting.evening", "Evening")
, ("common.greeting.noon", "Noon")
, ("...", "...")
]

So I think there's a couple of ways to go:

  1. fromList: Like you suggested but long and nested keys will be crappy to maintain.
  2. fromTree: This would require an API to build a proper translations structure but give you all the type safety you need, let you deal with nesting nicely only at the small cost of a little more verbosity. It could look something like this:
    -- imagine I18Next also exports object and string
    translations =
    I18Next.fromTree <|
        object
            [ ( "common"
              , object
                    [ ( "greeting"
                      , object
                            [ ( "morning", string "Morning" )
                            , ( "evening", string "Evening" )
                            , ( "afternoon", string "Afternoon" )
                            ]
                      )
                    ]
              )
            ]

What are your thoughts on this?

Tehnix commented 4 years ago

I think your I18Next.fromTree solution would probably be the way to go. It would be simple enough for non-nested translations, while allowing the full power if needed :)

My case would then simply be something like:

translations =
    I18Next.fromTree <|
        object
            [ ( "morning", string "Morning" )
            , ( "evening", string "Evening" )
            , ( "afternoon", string "Afternoon" )
            ]
ChristophP commented 4 years ago

@Tehnix Cool, then I think a couple of things need to happen.

  1. Implement string : String -> Tree as an alias for Leaf

  2. Implement object : List (String, Tree) -> Tree using Branch

  3. Implement fromTree : List (String, Tree) -> Translations, using foldTree and Translations. I would expect a list as input here. If it were just a Tree someone could pass a string value as a root object, which couldn't be handled by t. That would make fromTree look like this:

    translations =
    I18Next.fromTree
      [ ("custom"
        , object
            [ ( "morning", string "Morning" )
            , ( "evening", string "Evening" )
            , ( "afternoon", string "Afternoon" )
            ]
        )
      , ("hello", string "hello")
     ]
  4. Expose Tree, string, object, fromTree and write a tests checking if t return the correct values when given a translations value that is custom built.

Would you want to give the implementation a try? I can also do it but it'll likely be sometime next week till I find time.

Tehnix commented 4 years ago

I can do it, but I probably won't have time before next weeks weekend (ran out of vacation 😅 )

ChristophP commented 4 years ago

@Tehnix Alright you are welcome to grab that task. I am also wrapped up in something else currently, so let's see who gets to it first.

ChristophP commented 4 years ago

Closed and tracked in #21

ChristophP commented 4 years ago

@Tehnix I found some time to work it in this PR #22