formatjs / formatjs-old

The monorepo home to all of the FormatJS related libraries.
https://formatjs.io/
156 stars 53 forks source link

Polyfill Intl.NumberFormat causes errors with Intl.DateTimeFormat #655

Closed tmt96 closed 4 years ago

tmt96 commented 4 years ago

Which package? intl-unified-number-format

Describe the bug If I use both the intl and intl-unified-number-format packages, Intl.DateTimeFormat returns error.

To Reproduce Steps to reproduce the behavior: Create a node project with index.js:

const intl = require("intl");
global.Intl = intl;
require("@formatjs/intl-pluralrules/polyfill-locales");
require("@formatjs/intl-unified-numberformat/polyfill");
require("@formatjs/intl-unified-numberformat/dist/locale-data/en-NZ");

const options = {
  year: "numeric",
  month: "2-digit",
};
const formatter = new Intl.DateTimeFormat("en-NZ", options);
console.log(formatter.format(new Date("2016-03-28")));

Expected behavior Console prints out the correctly format date '03/2016'

Actual behavior Console prints out error:

/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:2247
        ild = data.symbols[nums] || data.symbols.latn,
                   ^

TypeError: Cannot read property 'symbols' of undefined
    at PartitionNumberPattern (/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:2247:20)
    at FormatNumber (/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:2486:17)
    at CreateDateTimeParts (/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:3987:26)
    at FormatDateTime (/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:4079:17)
    at DateTimeFormatConstructor.F (/Users/tuanmt/Programming/intl-test/node_modules/intl/lib/core.js:3848:20)
    at Object.<anonymous> (/Users/tuanmt/Programming/intl-test/index.js:12:23
pyrocat101 commented 4 years ago

@tmt96 what is your node.js version?

tmt96 commented 4 years ago

I'm on Node 10. However, we want to use NumberFormat in React Native on Hermes engine (https://hermesengine.dev/), which does not implement the Intl API and thus requires polyfill

tmt96 commented 4 years ago

If it's helpful, I did reproduce the bug with Node 13.1

tmt96 commented 4 years ago

@pyrocat101 do we have any update for this bug?

pyrocat101 commented 4 years ago

@tmt96 Hey, sorry busy at work on weekdays. I will get to it this weekend!

pyrocat101 commented 4 years ago

@tmt96 It looks like a bug on Intl.js side.

The CreateDateTimeParts implementation references Intl.NumberFormat that you polyfilled:

https://github.com/andyearnshaw/Intl.js/blob/4bd382c5df206f328b35ca23579904f603b84f0b/src/12.datetimeformat.js#L908

This nf2 is called by FormatNumber here: https://github.com/andyearnshaw/Intl.js/blob/4bd382c5df206f328b35ca23579904f603b84f0b/src/12.datetimeformat.js#L991

And this FormatNumber incorrectly assumes that Intl.NumberFormat is also polyfilled by Intl.DateTimeFormat and calls into PartitionNumberPattern, which is Intl.js internal implementation specific:

https://github.com/andyearnshaw/Intl.js/blob/4bd382c5df206f328b35ca23579904f603b84f0b/src/11.numberformat.js#L517


As for solutions, I think you can either use native Intl.DateTimeFormat, or use patch this line here to call nf2.format(v):

https://github.com/andyearnshaw/Intl.js/blob/4bd382c5df206f328b35ca23579904f603b84f0b/dist/Intl.js#L4309