Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intl.[ctor] called with unusual target produces strange behavior #3692

Closed
dilijev opened this issue Sep 8, 2017 · 4 comments
Closed

Intl.[ctor] called with unusual target produces strange behavior #3692

dilijev opened this issue Sep 8, 2017 · 4 comments

Comments

@dilijev
Copy link
Contributor

dilijev commented Sep 8, 2017

Found by reading code and wondering "what does this button do?"

Apparently Intl.* constructors do not actually have to be called with new, and can be called with anything as the target.

Using Intl or undefined as the target (first param of call) produces the same result as calling new.
Specifically allowing the target to be Intl or undefined (see Intl.js code, snippet below) allows for calling as Intl.NumberFormat and assigning the same to a var and then calling, producing the same results.

// function NumberFormat
if (this === Intl || this === undefined) {
    // after the call to the same function using new, this === NumberFormat, but any other value of this will be allowed past this check, leading to strange behavior
    return new NumberFormat(locales, options);
}
## Source
let options = {style:'currency', currency:'USD'};
print(new Intl.NumberFormat('en-us', options).format(42));
print(Intl.NumberFormat.call(Intl, 'en-us', options).format(42));
print(Intl.NumberFormat.call(undefined, 'en-us', options).format(42));

print(Intl.NumberFormat('en-us', options).format(42)); /* call(Intl, ...) */
nf = Intl.NumberFormat;
print(nf('en-us', options).format(42)); /* call(undefined, ...) */

┌──────────────────┬────────┐
│ ch-1.7.1         │ $42.00 │
│ ch-master-latest │ $42.00 │
│ d8               │ $42.00 │
│ jsc              │ $42.00 │
│ node             │ $42.00 │
│ node-ch          │        │
│ sm               │        │
└──────────────────┴────────┘

Using anything else apparently returns the object passed in:

## Source
let options = {style:'currency', currency:'USD'};
let a = Intl.NumberFormat.call({}, 'en-us', options);
print(a);
print(a.format(42));

┌──────────────────┬───────────────────────────────────────────────────────────────┐
│ d8               │ [object Object]                                               │
│ jsc              │ $42.00                                                        │
│ sm               │                                                               │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ ch-1.7.1         │ [object Object]                                               │
│ ch-master-latest │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node-ch          │ {}                                                            │
│                  │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node             │ NumberFormat {}                                               │
│                  │ $42.00                                                        │
└──────────────────┴───────────────────────────────────────────────────────────────┘
## Source
let options = {style:'currency', currency:'USD'};
let a = Intl.NumberFormat.call(function foo() {}, 'en-us', options);
print(a);
print(a.format(42));

┌──────────────────┬───────────────────────────────────────────────────────────────┐
│ d8               │ [object Object]                                               │
│ jsc              │ $42.00                                                        │
│ sm               │                                                               │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ ch-1.7.1         │ function foo() {}                                             │
│ ch-master-latest │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node-ch          │ [Function: foo]                                               │
│                  │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node             │ NumberFormat {}                                               │
│                  │ $42.00                                                        │
└──────────────────┴───────────────────────────────────────────────────────────────┘
## Source
let options = {style:'currency', currency:'USD'};
let a = Intl.NumberFormat.call(new (function foo() {}), 'en-us', options);
print(a);
print(a.format(42));

┌──────────────────┬───────────────────────────────────────────────────────────────┐
│ d8               │ [object Object]                                               │
│ jsc              │ $42.00                                                        │
│ sm               │                                                               │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ ch-1.7.1         │ [object Object]                                               │
│ ch-master-latest │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node-ch          │ foo {}                                                        │
│                  │ TypeError: Object doesn't support property or method 'format' │
├──────────────────┼───────────────────────────────────────────────────────────────┤
│ node             │ NumberFormat {}                                               │
│                  │ $42.00                                                        │
└──────────────────┴───────────────────────────────────────────────────────────────┘

Which leads to fun things like formatting a Date with something you would have hoped was a NumberFormat object

## Source
let a = Intl.NumberFormat.call(Intl.DateTimeFormat, 'en-us', { style: 'percent' });
print(a);
print((a.format || (new a).format)(new Date(2017,0,1)));


┌──────────────────┬────────────────────────────────────────────────────────────────────────┐
│ d8               │ [object Object]                                                        │
│ jsc              │ 148,325,760,000,000%                                                   │
│ sm               │                                                                        │
├──────────────────┼────────────────────────────────────────────────────────────────────────┤
│ ch-1.7.1         │ function DateTimeFormat() { [native code] }                            │
│ ch-master-latest │ ?1?/?1?/?2017                                                          │
├──────────────────┼────────────────────────────────────────────────────────────────────────┤
│ node-ch          │ { [Function: DateTimeFormat] __relevantExtensionKeys: [ 'ca', 'nu' ] } │
│                  │ ‎1‎/‎1‎/‎2017                                                          │
├──────────────────┼────────────────────────────────────────────────────────────────────────┤
│ node             │ NumberFormat {}                                                        │
│                  │ 148,325,760,000,000%                                                   │
└──────────────────┴────────────────────────────────────────────────────────────────────────┘
@dilijev
Copy link
Contributor Author

dilijev commented Sep 8, 2017

@bterlson I'm sure that our behavior here is simply incorrect, but I want to make sure we get all the nuances of the behavior in the fix for this. I'm having a hard time reading through the relevant spec to see what all the cases would be. https://tc39.github.io/ecma402/#sec-intl-numberformat-constructor

@bterlson
Copy link
Contributor

bterlson commented Sep 8, 2017

@dilijev what cases are you referring to? How can I help?

@dilijev
Copy link
Contributor Author

dilijev commented Sep 8, 2017

@bterlson basically the Intl.js code in this case doesn't read very much like the spec. Might need a 1:1 with you to go over terminology and walk through the code. This can wait since it's Backlog for now.

It might not be relevant here but I get kind of lost when I start seeing ReturnIfAbrupt notations.

Having difficulty with this line;

  1. If NewTarget is undefined, let newTarget be the active function object, else let newTarget be NewTarget.

That almost makes it sound like what we do in this case is some semblance of correct (maybe a little off) and that the other engines are wrong too, but I feel like that can't be right because the behavior makes no sense.

@jackhorton
Copy link
Contributor

I have wondered about our behavior here too... I am almost sure its wrong, but its not entirely clear how to be right from reading the spec.

@dilijev dilijev modified the milestones: Backlog, 1.10 May 31, 2018
@digitalinfinity digitalinfinity modified the milestones: 1.10, vNext Aug 1, 2018
chakrabot pushed a commit that referenced this issue Aug 28, 2018
…t262 issues, add support for ICU 62.1

Merge pull request #5612 from jackhorton:intl/lots-of-fixes

Fixes #637
Fixes #5603
Fixes #3427
Fixes #3513
Fixes #3692

Adds build support for ICU 62.1 by enabling /utf-8 in all ICU files rather than just i18n

@rhuanjl's function.length PR took our pass rate from 486 to 530; this further increases it to 580 (compared to V8's 582 and SpiderMonkey's 597). The remaining failures are all known, but slightly more complicated issues, where we shell out to ICU when the spec wants us to do something particularly snowflake-y -- V8 fails most of these tests as well. The remaining ~350 tests are for features that we don't implement and V8/SM either haven't implemented or have turned off by default.

For what its worth, some of the features we haven't implemented are extremely straightforward -- the Intl version of String.prototype.toLocale{Lower|Upper}Case and {Typed}Array.prototype.toLocaleString are tiny. Fixing those tests (by implementing those features) would bump us up to 596.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants