Thanks for the reply Evan, I’ll try to provide some more information - I was unsure how to go about this at all, especially as the work is not mine, and it looks like the original author currently has limited time to put into this.
After a very quick check, I think everything in the package is pure functions (I can do further checks, but there is no need for any of it to be non-pure, because what is wraps is entirely pure, except for a few functions that can raise exceptions if you pass in invalid values). Very little of the code is a ‘copy’ of JS - it is simply wrappers for the browser builtin package Intl (This library is not available in some older browsers, but there is a polyfill).
As explained in another thread, we really do not want to re-implement the entire library in pure Elm. This would likely be a massive effort (not to mention extremely boring), the result would be likely very slow and buggy, and it would require sending a huge amount of data over the wire (CLDR data from the Unicode consortium) - data that is built-in to modern browsers.
In terms of design/approach, I think this is one of the ‘boring’ bits of the web platform which is pretty straightforward to wrap, where mostly you should stick as close as possible to the JS API (so that people can transfer knowledge/docs with little friction), and add some Elm-ish improvements. My initial impression is that the current implementation does that already and looks very good.
To be more specific, the browser API (MDN) is that you create NumberFormat/DateTimeFormat objects, and then use methods on these to format a number in local aware way, for example. This API is already well designed for good performance - once you have a NumberFormat object, for example, you can keep on using that rather than having to look up the locale every time.
The Elm wrappers pretty much follow the same API, which allows for good performance while keeping them very simple. There are a few points where it does some obvious mappings into Elm:
-
Maybe for APIs that can return null or throw errors
- Where the JS API takes an
options object (which in JS land can have any number of attributes or none), the default options are exposed as a strongly typed record which can then be used as a basis for specifying non-defaults (like this)
- Simple union types for options that can take a few specific values.
In addition, it looks like the Locale object is an Elm specific innovation. Instead of passing strings like en-US to create a NumberFormat object, you first create a Locale object from en-US which validates it (and can therefore return Nothing), and then use this to create NumberFormat. This means that you get much better error handling and eliminate runtime errors that are possible with the JS API.
Formatted docs for the included modules can be found here:
http://elm-directory.herokuapp.com/package/vanwagonet/elm-intl/1.0.0/Intl.Locale
(I’ve spotted some issues in the docs already, these would be need to be cleaned up).
Is there someone else who could do the auditing/review of this to take the burden off you? I’m happy to do a full review of it myself and give a report, if that would help at all, I don’t know what your process is for this kind of thing. I could also go through the docs and submit issues/PRs for anything I find, and then get back to you.