Skip to content

Conversation

@FrankYFTang
Copy link
Contributor

The spec text change for https://github.com/tc39/proposal-intl-locale-info

We should discuss this once Intl Locale Info API is ready to move to stage 4.
This is one of the require task.

@sffc
Copy link
Contributor

sffc commented Dec 21, 2024

@ryzokuken ryzokuken marked this pull request as draft May 13, 2025 14:21
@FrankYFTang FrankYFTang moved this to Priority Issues in ECMA-402 Meeting Topics Oct 8, 2025
@sffc sffc moved this from Priority Issues to Previously Discussed in ECMA-402 Meeting Topics Oct 9, 2025
@sffc
Copy link
Contributor

sffc commented Oct 13, 2025

@FrankYFTang FrankYFTang marked this pull request as ready for review October 14, 2025 23:27
@FrankYFTang
Copy link
Contributor Author

Ready to be review for merging if Intl Locale Info advanced to Stage 4. Still need to merge tc39/proposal-intl-locale-info#92 into this PR first if TC39 approve both

@FrankYFTang
Copy link
Contributor Author

@ben-allen @gibson042 @ryzokuken will be nice if you can voice your concern or support about this PR from the editor's view ASAP before the TG1. Thanks

Copy link
Member

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things to fix before approval.

spec/locale.html Outdated
Comment on lines 396 to 397
1. Let _region_ be GetLocaleRegion(_loc_.[[Locale]]).
1. If _region_ is *undefined*, return *undefined*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic seems like it belongs in TimeZonesOfLocale, replacing the assertion and bringing it in better alignment with LocaleDirection.

         1. Let _region_ be GetLocaleRegion(_loc_.[[Locale]]).
-        1. Assert: _region_ is not *undefined*.
+        1. If _region_ is *undefined*, return *undefined*.
         1. Let _list_ be a List of unique canonical time zone identifiers, which must be String values indicating a canonical Zone name of the IANA Time Zone Database, of those in common use in _region_. The list is empty if no time zones are commonly used in _region_. The list is sorted according to lexicographic code unit order.
         1. Return CreateArrayFromList( _list_ ).
Suggested change
1. Let _region_ be GetLocaleRegion(_loc_.[[Locale]]).
1. If _region_ is *undefined*, return *undefined*.

Comment on lines +410 to +411
1. If _dir_ is not *undefined*, then
1. Perform ! CreateDataPropertyOrThrow(_info_, *"direction"*, _dir_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does test262 cover absence of a "direction" property in any case? I'd strongly prefer it to always be present on the returned object, rather than potentially falling through to Object.prototype (even though resolvedOptions output is also vulnerable to that issue).

Copy link
Contributor Author

@FrankYFTang FrankYFTang Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer it to always be present on the returned object

that will be a normative change, right?

Copy link
Member

@gibson042 gibson042 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's exactly what I'm getting at. On my machine, all implementations supporting Intl.Locale.prototype.getTextInfo (JavaScriptCore, LibJS, and V8) include a non-empty string-valued direction property for each of the 26**2 + 26**3 = 18252 language-subtag-only alpha{2,3} sequences—are you aware of a locale for which any of them would fail to exercise this CreateDataPropertyOrThrow step? Because if there isn't one, then I'd consider this a non-normative change that we should make now to avoid problems in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you asking me which locale that the (new Intl.Locale(locale))getTextInfo().direction would be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the spec, the following value should be undefined but V8 and Safari currently is not implemented correctly

(new Intl.Locale("und-Zyyy")).getTextInfo().direction
(new Intl.Locale("und-Zinh")).getTextInfo().direction
(new Intl.Locale("und-Zzzz")).getTextInfo().direction
(new Intl.Locale("und-Brai")).getTextInfo().direction

See discussion in tc39/proposal-intl-locale-info#101

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it seems to me like this change is still appropriate, especially since tc39/proposal-intl-locale-info#101 (comment) documents the TG1 decision as

The API getTextInfo can return an object with a field containing the string value "ltr", the string value "rtl", or undefined

(i.e., that the field should be present with value undefined rather than absent)

@ben-allen @ryzokuken what are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this kind of if x is not undefined CreateDataPropertyOrThrow pattern in many places

  1. all resolvedOptions() in ECMA402
  2. in ecma262
    https://tc39.es/ecma262/#sec-frompropertydescriptor
    https://tc39.es/ecma262/#sec-copydataproperties
    https://tc39.es/ecma262/#sec-object.getownpropertydescriptors

@FrankYFTang
Copy link
Contributor Author

PTAL
I adress your feedback
I also merge the last PR in the proposal

and in 2025-11-18 TC39 this stage 4 proposal is advanced to stage 4

@FrankYFTang
Copy link
Contributor Author

PTAL

FrankYFTang and others added 10 commits November 20, 2025 14:53
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Co-authored-by: Richard Gibson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Previously Discussed

Development

Successfully merging this pull request may close these issues.

3 participants