Replace 'localeCompare' with function independent from locale

[IvanGoncharov]

Fixes #2869

[backbone87]

naturalCompare is case sensitive, while localeCompare isnt. this leads to changes in schema dumps. its a breaking change, but more an annoyance than causing real issues. one could argue that sorting case sensitive is not as natural as one expects.

edit: i think it would be better to accept a comparator function for string comparison which defaults to Intl.Collator.prototype.compare.bind(new Intl.Comparator('en-US'))

[IvanGoncharov]

@backbone87 Thanks for the feedback 👍

Intl.Collator.prototype.compare.bind(new Intl.Comparator('en-US'))

Since we support a bunch of platforms Node/Deno/Different browser we don't want to depend on anything beyond core JS APIs.
Also, the main purpose of graphql-js is to be reference implementation so libraries in other languages can easily replicate our code by just converting to their language (e.g. whole GraphQL in Python ecosystem is using line by line port of this lib https://github.com/graphql-python/graphql-core#graphql-core-3) so we try not to use stuff that can't be easily ported.

i think it would be better to accept a comparator function for string comparison which defaults

Happy to do that if we discover a few popular use cases that can't be solved by the single compare function.

one could argue that sorting case sensitive is not as natural as one expects.

I'm open to this change if you think it provides better DX in popular cases.

naturalCompare is case sensitive, while localeCompare isn't. this leads to changes in schema dumps. its a breaking change, but more an annoyance than causing real issues.

Partly agree 👍 If it causes a major pain point for someone I can revert it on 15.x.x line.
One downside, in that case, is that 15.x.x and upcoming 16.x.x will produce different dumps so it would be hard for library authors that need to support both 15.x.x and 16.x.x through peer dependencies.

[backbone87]

Partly agree 👍 If it causes a major pain point for someone I can revert it on 15.x.x line.

it depends for what you use schema dumps for. in our case we use type-graphql and commit a dumped schema snapshot for easier review (which is checked by CI to be "clean"). updating to latest graphql-js@^15 changed the snapshot in non-semantical ways, which would appear as an "unrelated" change during review, for appearently unknown reason.

thats why i commented here for reference that maybe safes some others a few minutes of research.

I'm open to this change if you think it provides better DX in popular cases.

as a reader of schema dumps: i would say yes its more intuitive to have case insensitive order. as a programer looking at the complexity of naturalCompare: do we really want to reimplement collation rules?

so i would change my proposal: introduce a comparator argument to lexicographicSortSchema which defaults to naturalCompare, but can then be just changed to Intl.Collator.prototype.compare.bind(new Intl.Comparator('en-US'))