Replace 'localeCompare' with function independent from locale#2876
Replace 'localeCompare' with function independent from locale#2876IvanGoncharov merged 1 commit intographql:masterfrom
Conversation
90cffec to
b1bef95
Compare
b1bef95 to
76766c1
Compare
|
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 |
|
@backbone87 Thanks for the feedback 👍
Since we support a bunch of platforms Node/Deno/Different browser we don't want to depend on anything beyond core JS APIs.
Happy to do that if we discover a few popular use cases that can't be solved by the single compare function.
I'm open to this change if you think it provides better DX in popular cases.
Partly agree 👍 If it causes a major pain point for someone I can revert it on |
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.
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 |
Fixes #2869