Skip to content

[Master] Fix 11339; using localeCompare cause exception in window 2012 dueto Intl#11541

Merged
yuit merged 4 commits into
masterfrom
master_11339
Oct 14, 2016
Merged

[Master] Fix 11339; using localeCompare cause exception in window 2012 dueto Intl#11541
yuit merged 4 commits into
masterfrom
master_11339

Conversation

@yuit
Copy link
Copy Markdown
Contributor

@yuit yuit commented Oct 11, 2016

Fix #11339
@mhegazy @vladima

  • Port to release-2.0.5

Comment thread src/compiler/core.ts Outdated
* @param locales string of BCP 47 language tag or an array of string of BCP 47 language tag
* @param options an object of Intl.CollatorOptions to specify localeCompare properties
*/
export function localeCompare(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): number | undefined {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would rather we use comperStrings as the only place where we use localeCompare rather than returning undefined, and having every caller check for it

Comment thread src/compiler/core.ts Outdated
}

export function compareStrings(a: string, b: string, ignoreCase?: boolean): Comparison {
export function compareStrings(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): Comparison {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we ever use locale? If not I would remove that parameter.

Also can we just accentSensitivety everywhere? Seems the only place we do not use it is nav bar ordering, but do not think that we need to.. an I missing something?

@yuit yuit merged commit cd04aa9 into master Oct 14, 2016
@yuit yuit deleted the master_11339 branch October 14, 2016 16:36
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants