feat: Add Intl.NumberFormat ES2020 polyfill

[longlho]

fixes #553

[JakeChampion]

Thanks for this pull request @longlho :-)
I ran the tests on our CI and they look to fail on chrome 29 up to chrome 76, specifically it is the feature detect which is not working correctly when the polyfill is applied.

    -> Intl.NumberFormat passes the feature detect
       http://bs-local.com:9876/test?includePolyfills=yes&always=no&feature=Intl.NumberFormat
       false == true

[JakeChampion]

Thanks for this pull request @longlho :-)
I ran the tests on our CI and they look to fail on chrome 29 up to chrome 76, specifically it is the feature detect which is not working correctly when the polyfill is applied.

    -> Intl.NumberFormat passes the feature detect
       http://bs-local.com:9876/test?includePolyfills=yes&always=no&feature=Intl.NumberFormat
       false == true

[longlho]

any suggestion on how to repro (other than asking you to re-run in your pipeline)?

[longlho]

depends on formatjs/formatjs#1684

[JakeChampion]

@longlho now that formatjs/formatjs#1684 is merged, do you know how we bring the update into this pull-request?

[longlho]

yup ill tackle that today

[longlho]

fixed :)

[JakeChampion]

@longlho it looks like the feature detect in detect.js does not return true once the polyfill has been applied in Chrome.

Here is the test failure we saw for this in CI:

Failures:
- chrome 29.0 up to and including 76.0:
  -> Intl.NumberFormat passes the feature detect
      http://bs-local.com:9876/test?includePolyfills=yes&always=no&feature=Intl.NumberFormat
      false == true

[longlho]

Hmm how is the polyfill loaded btw? It needs to load the en locale for detect to pass. I don't wanna add en locale data to polyfill.js since it'll load that for every locale.

[JakeChampion]

Hmm how is the polyfill loaded btw? It needs to load the en locale for detect to pass. I don't wanna add en locale data to polyfill.js since it'll load that for every locale.

That is a good point, we should not load in the locale for en if it is not requested.

Perhaps we can have a less thorough feature detect like the other Intl polyfills we have? So instead of this:

'Intl' in self && 'NumberFormat' in self.Intl && (function () {
  try {
    new Intl.NumberFormat(undefined, {
      style: 'unit',
      unit: 'byte',
    });
  } catch (e) {
    return false;
  }
  return true;
})()

We have this:

'Intl' in self && 'NumberFormat' in self.Intl

Would that detect be enough or do we need to instantiate Intl.NumberFormat with a style?

[JakeChampion]

@longlho How about this feature detect instead?

'Intl' in self && 'NumberFormat' in self.Intl && (function () {
  try {
    var options = {
      compactDisplay: 'long'
    };
    var notationSupport = false;
    Object.defineProperty(options, 'notation', {
      get: function() {
        notationSupport = true;
        return 'compact';
      }
    });

    var compactDisplaySupport = false;
    Object.defineProperty(options, 'compactDisplay', {
      get: function() {
        compactDisplay = true;
        return 'long';
      }
    });

    new Intl.NumberFormat(undefined, options).format(1_807_687.733);

    return notationSupport && compactDisplaySupport;
    } catch (e) {
      return false;
    }
  return true;
})()

[longlho]

Let me double check our implementation but that might work. The issue might be that due to https://tc39.es/ecma402/#sec-initializenumberformat step 11 we try to resolve the locale 1st, and since there's no locale it might crap out.

[longlho]

Yeah it's technically not spec-compliant to not check for a locale, since it expects at least 1 locale (DefaultLocale) to be loaded...

[JakeChampion]

Let me double check our implementation but that might work. The issue might be that due to tc39.es/ecma402/#sec-initializenumberformat step 11 we try to resolve the locale 1st, and since there's no locale it might crap out.

Ah yes, I think I understand, for the detect to pass we must first insert some locale data into it. Let me see if I need to rework our test-runner to make that possible.

[JakeChampion]

Our test-runner creates a describe block which contains an it block for the feature detect tests and then it embeds the contents of the tests.js into the describe block as well.
https://github.com/Financial-Times/polyfill-library/blob/2e2fe842881e9b7b1359005c35db09c64666cc36/test/polyfills/server.js#L153-L162

This means we can write a beforeEach or before block which adds the locale data before the feature detect test is called. I'll update this PR with the change required.

[JakeChampion]

The tests all passed :-) https://github.com/Financial-Times/polyfill-library/runs/758174562

[longlho]

awesome thanks a lot @JakeChampion !