Change message and translation to record types

[cknitt]

@alexfedoseev Also, I moved the translation type from the ReactIntl module to the Util module, as it is actually not used anywhere but in Util.

Note that I still need to release a new version of the extractor to make message extraction from PageLocale.re work again. I will do that once we have this PR merged.

[ixzzd]

@cknitt Do you have plans to keep description field support?

[ixzzd]

Not related to current PR, but maybe there is a way to generate ids with the PPX too? It could be great

[cknitt]

Which PPX are you referring to? bs-react-intl-extractor is not a PPX and can only extract existing messages.

But yes, it might be nice to have a PPX that could extract a message with id PageLocale.hello and defaultMessage Hello from module PageLocale.re with

let hello = [%intl.msg "Hello"];

or something like that.

[ixzzd]

Which PPX are you referring to?

There is not such ppx yet, it exists only in my dreams :) But maybe I can find time to work on it. I an ppx noob, but I think it should be possible to use ppx-per file, not ppx-per-constant.

[@intl.someppx];
let hello = {defaultMessage: "Hello", description: None};
let world = {defaultMessage: "world", description: "world description"};

[alex35mil]

It can be

let a = [%intl.msg {id: "a", message: "message a"}];
let b = [%intl.msg
  {id: "b", message: "message b", description: "description b"}
];

Or

let a = [@intl.msg] {id: "a", message: "message a"};
let b = [@intl.msg]
  {id: "b", message: "message b", description: "description b"};

But the latter would produce a confusing error in case of a typo in an attribute.

[cknitt]

I would not specify the id explicitly, but just use <module name>.<variable name> as the id automatically.

Anyway, let's leave this for a possible future PR. 😉

[alex35mil]

👌 I think I saw ppx that does this id thing.

It can even be

let a = [%intl.msg "message a"];
let b = [%intl.msg {message: "message b", description: "description b"}];

[ixzzd]

@alexfedoseev this is great! But is it possible to use ppx only once at the beginning of the module?

[%intl.msgmodule]
let a = "message a";
let b = {message: "message b", description: "description b"};

For example, [@bs.config {jsx: 2}]; works for the whole module.

[alex35mil]

@ixzzd I don't think it's possible since PPX wouldn't capture outside things. I'm not sure how exactly [@bs.config {jsx: 2}] works but guessing it's an attribute that is used by the compiler to figure how to handle the current module, i.e. it's not handled by the PPX engine. The same way as the extractor consumes [@intl.messages].

[cknitt]

@cknitt Do you have plans to keep description field support?

@ixzzd I agree that it would be nice to have a way to support a description field (see also #5 which I opened quite some time ago 😉).

However, I could not really think of a good solution. I would not like to have to add description: None everywhere, as in

let hello = {id: "page.hello", defaultMessage: "Hello", description: None};

Do you have any suggestion for a better approach?

[ixzzd]

I would not like to have to add description: None everywhere

My opinion- it is better to have it everywhere than doesn't have it at all, but I don't see any better solution than yours :(

[alex35mil]

Looking great! One small comment.

[alex35mil]

It can be

let a = [%intl.msg {id: "a", message: "message a"}];
let b = [%intl.msg
  {id: "b", message: "message b", description: "description b"}
];

Or

let a = [@intl.msg] {id: "a", message: "message a"};
let b = [@intl.msg]
  {id: "b", message: "message b", description: "description b"};

But the latter would produce a confusing error in case of a typo in an attribute.

[alex35mil]

I would name this module Translation then:

// Translation.re
type t = ...;
let toDict = ...;

[alex35mil]

Looks perfect 👍