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

Should localeCompare return in Effect? #169

Open
pete-murphy opened this issue Jan 21, 2024 · 7 comments
Open

Should localeCompare return in Effect? #169

pete-murphy opened this issue Jan 21, 2024 · 7 comments

Comments

@pete-murphy
Copy link

pete-murphy commented Jan 21, 2024

localeCompare can return different output depending on locale. Example borrowed from MDN:

console.log("ä".localeCompare("z", "de")); // a negative value: in German, ä sorts before z
console.log("ä".localeCompare("z", "sv")); // a positive value: in Swedish, ä sorts after z

The FFI for Data.String.Common.localeCompare uses the system default locale, so Data.String.Common.localeCompare "ä" "z" could be LT or GT depending on if it's run in Germany or Sweden, for example.

It seems unlikely (maybe even impossible? I'm not sure) for a system's locale to change over the lifetime of a program, so maybe that is enough to consider localeCompare observationally pure?

I'm mostly just curious about the intuition for what makes FFI code "effect"-ful, as I have a library of bindings for the Intl object and I was planning on adding to it a localeCompare that accepted the Locale argument as well.

@garyb
Copy link
Member

garyb commented Jan 21, 2024

I agree, this does seem like it should be in Effect since it's based on the environment it's being run in.

@michaelficarra
Copy link
Contributor

I confirmed with the Intl authors that a JS program may observe the system locale change over the course of its execution, so localeCompare should indeed be considered impure.

@pete-murphy
Copy link
Author

pete-murphy commented Jan 25, 2024

I could make a PR to update the type here (along with the docs and tests), but looking at the rest of the library, this would be the first use of Effect outside of test code. I'm wondering if there's another discussion to be had around whether this function belongs in the library?

Tracking down when localeCompare was added, I was surprised to find it's been here since the beginning—a decade ago!

@michaelficarra
Copy link
Contributor

Well it's still useful when passed a locale explicitly. Maybe just require the locale parameter? Or is there maybe a more appropriate module for locale-sensitive operations to live in?

@pete-murphy
Copy link
Author

Or is there maybe a more appropriate module for locale-sensitive operations to live in?

I've been working on bindings for the locale-sensitive operations specified in ECMA 402 here: https://github.com/pete-murphy/purescript-js-intl/tree/main/src/JS/LocaleSensitive. From what I remember of last time I looked, I couldn't find any other references to any of those functions on Pursuit aside from this library's localeCompare.

Maybe just require the locale parameter?

What's the type of the locale parameter? If it's String then it can throw

"a".localeCompare("b", "zzzz") // throws a RangeError

Even if it were a well-typed Locale or Array Locale you'd have to account for the runtime not supporting localeCompare for that locale and falling back to the system locale.

@michaelficarra
Copy link
Contributor

What's the type of the locale parameter? If it's String then it can throw

That's a good point. Then I support dropping it or moving it to another more appropriate place.

@garyb
Copy link
Member

garyb commented Jan 25, 2024

I think it seems reasonable to drop it here 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants