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

Can we change the Intl prototypes to be normal objects? #122

Closed
anba opened this issue Jan 23, 2017 · 15 comments
Closed

Can we change the Intl prototypes to be normal objects? #122

anba opened this issue Jan 23, 2017 · 15 comments
Labels
editorial Involves an editorial fix question web reality

Comments

@anba
Copy link
Contributor

anba commented Jan 23, 2017

From https://bugzilla.mozilla.org/show_bug.cgi?id=1332604:

Intl.prototype.{Collator, DateTimeFormat, NumberFormat} are currently spec'ed as being instances of Intl.{Collator, DateTimeFormat, NumberFormat} (cf. 10.3, 11.4, 12.4). We should consider changing them to normal objects similar to how Date.prototype was changed to a normal object in the ECMAScript specification for ES6.

This change should be web-compatible because V8 already treats the prototype objects as normal objects.

@zbraniecki
Copy link
Member

@caridy , @domenic , @littledan, @jungshik

@littledan
Copy link
Member

Yep, let's do it!

@littledan
Copy link
Member

cc other implementations @jswalden @thetalecrafter @bterlson

@vanwagonet
Copy link

Intl.NumberFormat.prototype.format(1234.5) currently throws in V8, but returns "1,234.5" in JSC. iirc, I interpreted the v2 spec to mean that the prototype object is a properly initialized Intl.* object.

Given that it currently throws in V8, I'd be really surprised if there is code in the wild that relies on JSC's current behavior. I think it would be good to have the prototypes be plain objects.

I'm willing to make the change in JSC once the spec change is approved.

@vanwagonet
Copy link

I did a quick pass at what the implications of this change might include:

Referencing Intl.Collator.prototype.compare, Intl.{DateTime,Number}Format.prototype.{format,formatToParts}, will throw an error. I think this makes sense, since they are defined as getters meant to be called on a properly initialized Intl object.

It's possible there is an expectation to be able to do Intl.Collator.prototype.compare.call(someCollator, a, b), but the spec has always been for getters of bound functions, so it wouldn't have used comeCollator's options anyway. This does make the change a little less analogous to Date.prototype, though.

Calling Intl.*.prototype.resolvedOptions() will throw an error. The only use case I could see, is to try to get the base defaults, which you can already do by creating an Intl.* object without passing any arguments.

@vanwagonet
Copy link

@caridy caridy added question editorial Involves an editorial fix labels Mar 17, 2017
littledan added a commit to littledan/proposal-intl-plural-rules that referenced this issue Mar 18, 2017
We are considering removing the way that other Intl prototypes are
instances in tc39/ecma402#122 , so it seems
like we can and should make this change at least for new proposals.

Closes tc39#29
littledan added a commit to littledan/ecma402 that referenced this issue Jun 28, 2017
littledan added a commit to littledan/test262 that referenced this issue Jun 28, 2017
@rwaldron
Copy link
Contributor

@ericf ping

@caridy
Copy link
Contributor

caridy commented Aug 10, 2017

@thetalecrafter it seems that your ticked was resolved.

I will like to bring this up in the next meeting, and try to get consensus to merge #148. I wonder what's @bterlson's position considering that Chakra is not throwing for Intl.NumberFormat.prototype.format(1234.5).

@vanwagonet
Copy link

@caridy Yeah the ticket was closed in April. The change is in the current technology preview and should be in Safari 11.

@rwaldron
Copy link
Contributor

@caridy Is this no longer an issue in Intl.js?

@caridy
Copy link
Contributor

caridy commented Aug 10, 2017

@rwaldron it is not longer an issue there:

> Intl.NumberFormat.prototype.format(1234.5)
TypeError: `this` value for format() is not an initialized Intl.NumberFormat object.
    at NumberFormatConstructor.GetFormatNumber (/Users/cpatino/repo/Intl.js/lib/core.js:2431:70)
    at repl:1:28

@caridy
Copy link
Contributor

caridy commented Aug 10, 2017

I have confirmed that V8, SpiderMonkey, Intl.js and JSC (technology preview) are aligned with this. I have opened the issue for ChakraCore as instructed by @bterlson for the fix.

It seems that we are all in agreement.

@leobalter
Copy link
Member

Do we need to bring this to the meeting or are we all set here? We have tests ready to land.

@littledan
Copy link
Member

Sounds like it's ready for @caridy to land. The tests can be landed once the spec patch is landed.

@littledan
Copy link
Member

I merged the PR based on the consensus that @caridy mentioned above, so tests should be good to land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Involves an editorial fix question web reality
Projects
None yet
Development

No branches or pull requests

7 participants