node 12.11.0: Scientific & Engineering notation NumberFormat bug

[DerekNonGeneric]

• Version: v12.11.0

• Platforms:

  • Windows Server 2019 x64

    v10.0.17763.737
    

  • Debian via Windows Subsystem for Linux (WSL)

    Linux WIN-RB75RCSBNFD 4.4.0-17763-Microsoft #379-Microsoft Wed Mar 06 19:16:00 PST 2019 x86_64 GNU/Linux
    

• Subsystem: icu

  {
    node: '12.11.0',
    v8: '7.7.299.11-node.12',
    uv: '1.32.0',
    zlib: '1.2.11',
    brotli: '1.0.7',
    ares: '1.15.0',
    modules: '72',
    nghttp2: '1.39.2',
    napi: '5',
    llhttp: '1.1.4',
    http_parser: '2.8.0',
    openssl: '1.1.1c',
    cldr: '35.1',
    icu: '64.2',
    tz: '2019a',
    unicode: '12.1'
  }
  

---

This was tested in the REPL with PowerShell on Windows and Bash on WSL Debian. They both have the same problem.

IN

const formatter = Intl.NumberFormat('en', {style: 'unit', unit: 'meter-per-second', notation: 'scientific'});

or

const formatter = Intl.NumberFormat('en', {style: 'unit', unit: 'meter-per-second', notation: 'engineering'});

and

formatter.format(299792458);

OUT

TypeError: Internal error. Icu error.

This is the same code used in the Intl.NumberFormat V8 docs.

This bug does not only arise when specifying notation as compact notation works without error.

[targos]

I don't know if throwing an error is spec-compliant, but I figured that the "unit" option requires full-icu to work properly.

/cc @nodejs/intl @nodejs/v8

[srl295]

it may be that the unit data is omitted from the 'small icu' default set and it should now be included now that units are present.

[targos]

@srl295 should I try to change this line?

node/tools/icu/icu_small.json

Line 32 in ccb524f

"unit": "none"

[srl295]

@srl295 should I try to change this line?

node/tools/icu/icu_small.json

Line 32 in ccb524f

"unit": "none"

Yes. And then rerun configure with --with-intl=small-icu --download=all --with-icu-source=https://github.com/unicode-org/icu/releases/download/release-64-2/icu4c-64_2-src.tgz to reapply the adjusted trimming.

[DerekNonGeneric]

I just ran these tests on the official amd64/node:current-buster Docker image after updating to use v12.11.0 and can confirm that this bug is widespread and not exclusive to Windows.