santhoshtr / cldrpluralruleparser Goto Github PK

Hi Santhosh,

Timo (@Krinkle) has commented about your library on our jQuery (cc @jzaefferer @scottgonzalez) recent talk about Globalize / CLDR.

Follow some general comments I have about it below.

PS: Please, forgive me for opening an issue to talk about it. If you prefer to have this conversation using another channel, just let me know.

In a brief, first I would like to understand some architect decisions made here, and second to point out suggestions, and a possible cooperation.

1. CLDRPluralRuleParser

1.1. LDML vs JSON

I've noticed demos are extracting the plural information from the LDML file. Any reason why not using the official CLDR JSON instead, and avoid this?

Suggestion

Unicode officially distributes CLDR in the JSON format [1] [2]. Using JSON in javascript is more straightforward, requiring simpler algorithms to manipulate it.

You could use this lightweight library https://github.com/rxaviers/cldr to help with the JSON CLDR manipulation, which is what we use in jQuery Globalize.

This whole block would become this:

var locale = new Cldr( localeStr );
var rules = locale.supplemental("plurals-type-cardinal/{language}");
/**
 * Example for "en":
*      {
*        "pluralRule-count-one": "i = 1 and v = 0 @integer 1",
*        "pluralRule-count-other": " @integer 0, 2~16, 100, 1000, 10000, 100000, 1000000, … @decimal 0.0~1.5, 10.0, 100.0, 1000.0, 10000.0, 100000.0, 1000000.0, …"
*      }
 */

1.2. API

I've noticed the API requires user (I'm naming user the code that uses this library) to pass the rule as an argument on user-code-level: https://github.com/santhoshtr/CLDRPluralRuleParser/blob/master/demo/demo.js#L50

Suggestions

First of all, congrats for making the library succinct, so all it does is what it's supposed to do: handle plural CLDR rules. Although, this is still a cumbersome boilerplate code to reside in the user code.

Usually, user needs to know what's the cardinal form given a locale, and a number. Something like this:

plural("en", 1) -> one
plural("en", 3) -> other

If you consider using CLDR in the JSON form, and using cldr.js, it would be piece of cake to offer an API like the above with very few extra code.

1.3. UMD

If it works on browsers, why not supporting AMD as well on your UMD wrapper?

Suggestions

https://github.com/umdjs/umd/blob/master/returnExports.js

2. Cooperation

Globalize and moment.js (cc @ichernev) are working the migration to CLDR. To make a long story short, moment.js needs plural support on parts of its code (eg. https://github.com/moment/moment/blob/develop/moment.js#L713-L726). Although I have not consulted any of the parts, perhaps you both could cooperate.

Hello @santhoshtr,

Are you going to tag 1.1.3 as well? Having it tagged makes bower life easier.

Thanks

From #6
If it works on browsers, why not supporting AMD as well on your UMD wrapper?

Suggestions

https://github.com/umdjs/umd/blob/master/returnExports.js

I'm trying to upgrade cldrpluralruleparser to v1.5.0 but the project is crashing.
If I import it like this: import pluralRuleParser from 'cldrpluralruleparser'; I have the following error (see the screenshot):

But if I import it from my node_modules everything works fine: import pluralRuleParser from '../node_modules/cldrpluralruleparser/src/CLDRPluralRuleParser';.

I have this issue only with v1.5.0 with node v20.6.0, with v1.4.0 everything works fine.

Please advise! Thanks!

There's a typo in the client script name: bin/cldrpluraltuleparser.js
This makes the package uninstallable from NPM.

It looks like cldr-data is a peerDependency, but it is not being used anywhere (that I can see).

I'm using this library with a bunch of other in my web page, and the JavaScript files are minified and concatenated. This library is in the middle, between other libraries.

The error I get in Firefox 113.0.2 is:

Uncaught SyntaxError: export declarations may only appear at top level of a module

The error I get in Chromium 112.0.5615.49 is:

Uncaught SyntaxError: Unexpected token 'export'

The source code in this library where the error is reported by both browsers:

export default pluralRuleParser; 

This is a regression from 1.3.2, which does not exhibit the problem.

hi, passing 'ru' I got true for both 'other' and 'many' is that intentional?

testing against number 5

false pluralRule-count-one v = 0 and i % 10 = 1 and i % 100 != 11 @integer 1, 21, 31, 41, 51, 61, 71, 81, 101, 1001, …
false pluralRule-count-few v = 0 and i % 10 = 2..4 and i % 100 != 12..14 @integer 2~4, 22~24, 32~34, 42~44, 52~54, 62, 102, 1002, …
true pluralRule-count-many v = 0 and i % 10 = 0 or v = 0 and i % 10 = 5..9 or v = 0 and i % 100 = 11..14 @integer 0, 5~19, 100, 1000, 10000, 100000, 1000000, …
true pluralRule-count-other    @decimal 0.0~1.5, 10.0, 100.0, 1000.0, 10000.0, 100000.0, 1000000.0, …

but i should only get true for many, or not?

thanks in advance.

There is apparently no minified fiie in the npm package. This means that CDNs such as jsdelivr.com and unpkg.com can't provide the minified file with a SHA-256 hash from the src directory, though they do provide a minified file with no hash.