Refactor for more type safety

[seanf]

Sorry, this proved a little larger than expected!

It's mostly refactoring to avoid a cast, to separate JSX from defineMessages and from formatMessage (fewer ifs), and to use more meaningful names.

But to avoid casting the PropertyAssignment's PropertyName as any I had to use text instead of escapedText (I think escapedText must be non-public compiler API), so we now use the un-escaped text as the message key (which seems like the right thing to do).

Also, I changed things a bit so that the object keys for defineMessages/formatMessage can now be string/number literals, not just identifiers.

It should be mostly logical if you read the commits in order, but let me know if you want it split up or something.

[seanf]

Also added tslint and followed most of its suggestions.

[BANG88]

@seanf do you mind that i invite you as a collaborator ?

[BANG88]

@seanf Thank you so much. Awesome refactor..

[seanf]

@seanf Thank you so much. Awesome refactor..

Thanks!

@seanf do you mind that i invite you as a collaborator ?

@BANG88 not at all! But please give me some guidance on what I should do (eg with my own code, or with others' pull requests). I think it's always good to have a second person review any code.

[seanf]

@BANG88 How would you feel about exposing the Message interface as the return type for the (publicly exported) main method (well, an array of them to be exact)? (As opposed to an array of plain object {}.)

It shouldn't break any consumers, because I believe it's a narrower type, but I'm still very new to TypeScript, so I could be wrong about that. In practice the consumers seem to be plain JavaScript npm scripts (unless they use ts-node), so they probably don't care about types. Still, I like types as documentation.

[BANG88]

I like the types too. If you prefer or think is useful for you please do it.

[seanf]

Thinking about it a bit more, I think it would be better if the interface looked more like this:

interface MessageDescriptor {
  id: string
  defaultMessage: string
  description?: string
  [key: string]: string; // for any other properties
}

Otherwise exposing it won't really add much value.

[seanf]

@BANG88 I wasn't sure if idiomatic TypeScript required "I" interfaces, but the guidelines for TypeScript itself advise against them, so I'm inclined to disallow the I prefix (and remove this TODO comment). Any objection?

[BANG88]

No. this just some coding style not strict rule. for the simple package we do not want too much configurations.

[seanf]

@BANG88 Well, tslint is going to be strict about this rule. I think I could disable the rule entirely, but I think it's important to be consistent, one way or another. Having some interfaces with I and some without is confusing.

[baktun14]

@seanf Hey great refactor! <3