formatjs / dust-intl

Dust helpers for internationalization.
http://formatjs.io/dust/
Other
48 stars 11 forks source link

when a timestamp is passed as string, it fails #25

Open sfj2 opened 10 years ago

sfj2 commented 10 years ago

the @intlDate only works if the timestamp is passed as a number, doesn't work if if it's a string, due to this it's almost impossible to use this, since when we have to pass some data to the helper, we can only do it as a string -

eg. - {@intlDate timeZone="UTC" val=12344555555 /} - works

{@intlDate timeZone="UTC" val="{mydata.sometimestamp}" /} - fails -

{@intlDate timeZone="UTC" val={mydata.sometimestamp} /} - can't do this since dust fails saying it can't match the brackets

ericf commented 10 years ago

So you're saying that Dust only supports data that's a String being passed to helpers?

fhemberger commented 10 years ago

Demo:

var tmpl = '<time>{@intlDate val={ts} /}</time>';
var ctx = { ts : (new Date()).getTime() };
dust.renderSource(tmpl, ctx, function(err, out) { console.log(out); });
// Dust error message:
// SyntaxError: Expected buffer, comment, partial, raw, reference,
// section or special but "{" found.
fhemberger commented 10 years ago

@ericf You also have a similar issue with one example from your README:

Template: <b>{@intlNumber val=40000.004 /}</b> Expected: <b>40,000.004</b> Actual: <b>40,000.4</b>

To get the correct result, you have to pass val as string: <b>{@intlNumber val="40000.004" /}</b>

drewfish commented 10 years ago

Does {@intlDate timeZone="UTC" val=mydata.sometimestamp /} work?

fhemberger commented 10 years ago

This works for me, but for verbosity you may want to consider the change I proposed in https://github.com/yahoo/dust-helper-intl/pull/26#issuecomment-46088679.

sfj2 commented 10 years ago

@drewfish that seems to work. thanks!. but it might be good to fix this to accept string too.

redonkulus commented 10 years ago

@sfj2 I'm not sure its needed. That just how Dust works, you don't need the quotes or brackets to pass in context values.

sfj2 commented 10 years ago

Agree @redonkulus. Maybe then we should fix the readme at least? this is what's there in the readme right now -

var timeStamp = (new Date()).getTime(); // 1395872439650 var tmpl = '';

and maybe we should have an example of {@intlDate timeZone="UTC" val=mydata.sometimestamp /} too? i talked to at least 3-4 people who works regularly on dust, and couldn't figure this out :)

redonkulus commented 10 years ago

Ya I'd say an example like you mention would be useful. This seems to trip up developers more often than we would like.

fhemberger commented 10 years ago

@sfj2 @redonkulus Just look in the PR #26, I've already proposed some fixes for the examples in the README and a note about timestamps. This should do the trick.

ericf commented 10 years ago

This relates to #35