Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Formatting month - value of long falling back to short #179

Closed
Arturszott opened this issue May 17, 2016 · 13 comments
Closed

Formatting month - value of long falling back to short #179

Arturszott opened this issue May 17, 2016 · 13 comments

Comments

@Arturszott
Copy link

Arturszott commented May 17, 2016

Example failing test case:

assert(new IntlPolyfill.DateTimeFormat('en', {
    month:'long',
    year: 'numeric'
}).format(new Date('2016/01/16')), 'January 2016');

expected value January 2016 but the actual value is Jan 2016

I've been digging it for a while and it looks like bestFormat is not the best :)

Cheers!

@caridy
Copy link
Collaborator

caridy commented May 17, 2016

@Arturszott BestFitFormatMatcher is not defined in the spec text, it is basically the implementation hook for browsers to do their own thing. we have introduced very recently, and the goal is to make the appropriate changes in that algo to match browsers. hopefully I will get people to help on that :)

In any case, I will take a look at this. At first glance, it seems that https://github.com/andyearnshaw/Intl.js/blob/master/src/12.datetimeformat.js#L609-L734 algo is not choosing the right pattern, which is clearly available as yMMMM here https://github.com/andyearnshaw/Intl.js/blob/master/locale-data/json/en.json#L65, we will have to make the right adjustments there to guarantee that.

@kwarr128
Copy link

kwarr128 commented May 17, 2016

This also seems to be happening for weekday as well, it's always falling back to short in my tests.

And year seems to always be forced to the full 4 digits (though i guess this may have been intentional, 2 digit years are kind of strange and can only makes sense in a m/d/y case anyway, i just know that you could do it before the recent release)

@watilde
Copy link
Contributor

watilde commented May 17, 2016

@caridy Is the yMMMM pattern we should cover like the following?

console.log(
  new IntlPolyfill.DateTimeFormat(
    "en", {
      year: "numeric",
      month: "long"
    }
  ).format(new Date('January 15'))
)
// => January 2015

At first, we can make algo of BestFitFormatMatcher the right pattern IMHO. Either way, it doesn't support yet, but this change could fix this issue: watilde@045af63
Diff: master...watilde:patch/fix-BestFitFormatMatcher

@mkohlmyr
Copy link

Using Intl.js 1.2.4 on NodeJS v4.4.5.

> const intl = require("intl")
undefined
> intl
{ getCanonicalLocales: [Function],
  __localeSensitiveProtos: 
   { Number: { toLocaleString: [Function] },
     Date: 
      { toLocaleString: [Function],
        toLocaleDateString: [Function],
        toLocaleTimeString: [Function] } } }

> new intl.DateTimeFormat('sv', {"weekday": "short"}).format(new Date("08/14/2016"))
'sön'
> new intl.DateTimeFormat('sv', {"weekday": "long"}).format(new Date("08/14/2016"))
'sön' <-- should be söndag

> new intl.DateTimeFormat('dk', {"weekday": "short"}).format(new Date("08/14/2016"))
'So.'
> new intl.DateTimeFormat('dk', {"weekday": "long"}).format(new Date("08/14/2016"))
'So.' <-- should be Søndag

Some further examples

> new intl.DateTimeFormat('es', {"month": "long"}).format(new Date("08/14/2016"))
'ago.' <-- should be agosto

> new intl.DateTimeFormat('no', {"month": "long"}).format(new Date("08/14/2016"))
'Aug.' <-- should be August

> new intl.DateTimeFormat('sv', {"month": "long"}).format(new Date("10/12/2016"))
'okt.' <-- should be oktober

> new intl.DateTimeFormat('sv', {"weekday": "narrow"}).format(new Date("10/12/2016"))
'ons' <-- should be O

> new intl.DateTimeFormat('en', {"weekday": "narrow"}).format(new Date("10/12/2016"))
'Wed' <-- should be W

@caridy
Copy link
Collaborator

caridy commented May 31, 2016

yeah, single entry matches are not a thing in CLDR, it seems that browsers are doing their own thing when it comes to format one element, we will have to add that into data or a custom resolution for that in BestFitFormatMatcher, PRs are welcome :)

@juandopazo
Copy link
Contributor

This looks like a regression. It was working in version 1.1.0.

http://jsbin.com/rusesaweta/edit?html,js,output

@caridy
Copy link
Collaborator

caridy commented Jun 3, 2016

yeah, but it broke many other things from 1.0.0. I have discussed this extensibly with @ericf, and now we just need help to tune-in the BestFitFormatMatcher algo, which is implementation specific.

steida added a commit to este/este that referenced this issue Jun 12, 2016
Because now server incorrectly renders short date when long is defined
- andyearnshaw/Intl.js#179

Will be fixed soon, but people can be confused with “React attempted to
reuse markup in a container but the checksum was invalid.”
@antoinerousseau
Copy link

antoinerousseau commented Jun 12, 2016

using formatMatcher: 'basic' in react-intl while waiting for a fix :)

@watilde
Copy link
Contributor

watilde commented Jun 13, 2016

@caridy Can't we use the algorithm as written in the specification? I tried with the following commit and it worked for this issue case: 045af63

@caridy
Copy link
Collaborator

caridy commented Jun 13, 2016

@watilde the BestFitFormatMatcher is not specified in 402, it is implementation specific. That's why it is free form. In the past, it was a combination of both methods in a single function, now that it is separated, we can experiment with BestFitFormatMatcher to match browsers.

@moll
Copy link

moll commented Jun 17, 2016

Yay. Does the merge above that closes this also cover the long weekday issues @mkohlmyr brought up?

@caridy
Copy link
Collaborator

caridy commented Jun 17, 2016

@moll no, that's a different issue, I have opened a case for it #190. @watilde can you take a look?

@caridy
Copy link
Collaborator

caridy commented Sep 20, 2016

https://github.com/andyearnshaw/Intl.js/releases/tag/v1.2.5

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants