perf: use Intl.getCanonicalLocale #2244

VIKTORVAV99 · 2024-10-13T12:57:01Z

Since this code was created 9 years ago there has been standardisation around this and it's now part of the Intl object.

Replacing this should have no impact on support as it has had full browser support for several years now: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales

This reduces the size by 519 bytes (49 096 bytes -> 48 577 bytes) and should be faster as it has a native implementation.

Checklist

only relevant code is changed (make a diff before you submit the PR)
run tests npm run test
tests are included
commit message and code follows the Developer's Certification of Origin

coveralls · 2024-10-13T12:58:47Z

coverage: 96.174% (-0.03%) from 96.199%
when pulling 80cda93 on VIKTORVAV99:replace_custom_language_formatting_code_with_Intl.getCanoncialLocales
into 60e3779 on i18next:master.

adrai · 2024-10-13T14:05:08Z

as long as there is V3 support that does not use the Intl api => https://github.com/i18next/i18next/blob/master/src/PluralResolver.js#L99 we can't merge this...
but we can consider this like (#2184) for the next major version

VIKTORVAV99 · 2024-10-13T14:15:32Z

as long as there is V3 support that does not use the Intl api => https://github.com/i18next/i18next/blob/master/src/PluralResolver.js#L99 we can't merge this... but we can consider this like (#2184) for the next major version

Intl.getCanonicalLocale is a lot older than plural rules so even then it should be supported IMO. But yeah probably best to put it in a new major version. Might I also suggest removing the lowerCaseLanguage option when that is done? (this one: this.options.lowerCaseLng I don't think it's adding any value).

…ge code, like suggested in #2244

adrai · 2024-10-13T14:18:23Z

Just released v23.16.0 that will use Intl.getCanonicalLocale if available...

adrai · 2024-10-13T14:23:04Z

regarding the lowerCaseLng... this was a user request long time ago... and part of i18next since v1.2.3... isn't it @jamuhl ?

VIKTORVAV99 · 2024-10-13T14:23:53Z

Just released v23.16.0 that will use Intl.getCanonicalLocale if available...

Just as a FYI, it would be really nice if you used PRs so other people like myself can watch for new PRs. Commits to master does not have this capability.

VIKTORVAV99 · 2024-10-13T14:28:00Z

I would for example have loved to provide some feedback as the typeof !== 'undefined' checks are not needed in this case as a simple truthy check would suffice. typeof functions and comparisons are slower than a simple true check.

adrai · 2024-10-13T14:32:37Z

Just as a FYI, it would be really nice if you used PRs so other people like myself can watch for new PRs. Commits to master does not have this capability.

you're right... next time..

I would for example have loved to provide some feedback as the typeof !== 'undefined' checks are not needed in this case as a simple truthy check would suffice. typeof functions and comparisons are slower than a simple true check.

Unfortunately, in some environments those truthy checks did not work... we experienced this also here: https://github.com/i18next/i18next/blob/master/src/i18next.js#L94

All these different environments and setups etc... are the reason why we want to keep the code compatibility as wide as possible... and not always use the newest syntax features etc...

VIKTORVAV99 · 2024-10-13T14:42:22Z

Just as a FYI, it would be really nice if you used PRs so other people like myself can watch for new PRs. Commits to master does not have this capability.

you're right... next time..

I would for example have loved to provide some feedback as the typeof !== 'undefined' checks are not needed in this case as a simple truthy check would suffice. typeof functions and comparisons are slower than a simple true check.

Unfortunately, in some environments those truthy checks did not work... we experienced this also here: https://github.com/i18next/i18next/blob/master/src/i18next.js#L94

All these different environments and setups etc... are the reason why we want to keep the code compatibility as wide as possible... and not always use the newest syntax features etc...

I'm really curious to hear about which environments have defined internal objects that resolve truthy if they don't exist. If those exist they are not actually JavaScript compliant and I don't think anyone should go out of their way to support them.

And while I agree that the newest syntax should not always be used I'd say it's fair game to use any feature that is supported by all supported browsers and engines. If end users want to support older stuff than that I'd say it's up to them to enable that either by using older versions of the package or polyfiling/transpiling the code.

It should not be up to you as a maintainer to support 10 year old browsers.

But that's just my take on it.

jamuhl · 2024-10-13T15:12:58Z

not all companies are fast movers - most are not, most big companies are not...locking them out just because of the latest greatest can be done - but should be done wisely (there is nothing worse than getting major versions every few months - breaking your codebase)...

adrai · 2024-10-13T15:16:57Z

It should not be up to you as a maintainer to support 10 year old browsers.

i18next is not only used in browsers... but also react-native, hexo, server side, meteor... etc...

adrai · 2024-10-13T15:23:52Z

btw: most users will just downgrade to still continue to work with i18next... and if they find some bugs or missing functionality etc... we are then forced to address that also on older i18next versions... more work for everyone, just because of new syntax etc...

VIKTORVAV99 · 2024-10-13T15:41:01Z

not all companies are fast movers - most are not, most big companies are not...locking them out just because of the latest greatest can be done - but should be done wisely (there is nothing worse than getting major versions every few months - breaking your codebase)...

That's the great part about pinning dependencies. Everything would work exactly as it does now for them. I don't see a problem here.

If they only use the latest all the time that's on them.

i18next is not only used in browsers... but also react-native, hexo, server side, meteor... etc...

Intl is supported in react-native, node, deno and other JS runtimes as well and have been for a long time.

It reached baseline support in September 2017.

btw: i18next/react-i18next#1797 to still continue to work with i18next... and if they find some bugs or missing functionality etc... we are then forced to address that also on older i18next versions... more work for everyone, just because of new syntax etc...

Which is my point, if they need to downgrade, from a major version they shouldn't have updated in the first place. Major versions indicate breaking changes and therefore extra care should be taken to ensure it works. Or to pin the dependency to the last supported version.

Newer applications and users should not have to suffer from bloated code or performance hits just because unsupported environments don't support a built in object or newer syntax.

For example i18next is 48.9 kb minified while Lingui is 7 kb, that is a lot of extra stuff for two libraries that aim to do the same thing.

adrai · 2024-10-13T15:55:47Z

Newer applications and users should not have to suffer from bloated code or performance hits just because unsupported environments don't support a built in object or newer syntax.

If bloated code or performance really hits in real world scenarios, I'm full ok with removing stuff for the newer (more modern) environments... but if it only brings a marginal improvement, then it's better not to cut off the user base straight away...

For example i18next is 48.9 kb minified while Lingui is 7 kb, that is a lot of extra stuff for two libraries that aim to do the same thing.

i18next has a completely different approach compared to Lingui... and btw: what are 50kb vs 7kb when your application loads mbs of data...?

But in general, I'm with you...
I already told you... for the next major, we will remove/change some stuff here...

VIKTORVAV99 · 2024-10-13T16:08:52Z

i18next has a completely different approach compared to Lingui... and btw: what are 50kb vs 7kb when your application loads mbs of data...

Because we built a SPA that bundles all the languages and core libraries and only connect to our backend to get production data, not translations. But it's also an indicator that there is a lot more operations that are going on than there needs to be, which I'm more concerned about.

More importantly our app bundles are split up and i18next is included in the core bundle as it's used everywhere in our application. The core bundle is currently 286.85 kB so removing 43 kb from that (excluding any extra reductions from the react package) would mean lowering our index bundle by 15%. Since that bundle is blocking all other code from executing that is a pretty significant change.

jamuhl · 2024-10-13T16:57:01Z

Sure smaller is better in general...but does your SPA really suffer from those 14kB i18next adds (if loaded gzipped)? Or could the time be invested better somewhere else?

I mean our app has a core module size of 900kB which gets cached after first load - and even if not - takes like 70ms to load. But we might be lucky our app is not used in environments having bad internet.

VIKTORVAV99 · 2024-10-13T17:19:19Z

Sure smaller is better in general...but does your SPA really suffer from those 14kB i18next adds (if loaded gzipped)? Or could the time be invested better somewhere else?

I mean our app has a core module size of 900kB which gets cached after first load - and even if not - takes like 70ms to load. But we might be lucky our app is not used in environments having bad internet.

Our app is very much used in mobile envs and there is for sure other optimisations that can be done (and are being done) but these are outside the index bundle. So while they are important and still are being done they don't have the same impact on first load as the core packages like i18next.

jamuhl · 2024-10-14T05:56:15Z

my last 50cents...looks like such changes have a wider impact as estimated...i18next/react-i18next#1797 (comment) comes to mind

if such changes break installments - I would consider not publishing this as just "patch" versions - but possible breaking major

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [i18next](https://www.i18next.com) ([source](https://github.com/i18next/i18next)) | dependencies | minor | [`23.15.2` -> `23.16.0`](https://renovatebot.com/diffs/npm/i18next/23.15.2/23.16.0) | --- ### Release Notes <details> <summary>i18next/i18next (i18next)</summary> ### [`v23.16.0`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#23160) [Compare Source](i18next/i18next@v23.15.2...v23.16.0) - use `Intl.getCanonicalLocales` function if available to format language code, like suggested in [2244](i18next/i18next#2244) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/221 Reviewed-by: Alexandre Soro <code@soro.dev> Co-authored-by: renovate <renovate@git.tristess.app> Co-committed-by: renovate <renovate@git.tristess.app>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [i18next](https://www.i18next.com) ([source](https://github.com/i18next/i18next)) | dependencies | major | [`23.16.8` -> `24.2.0`](https://renovatebot.com/diffs/npm/i18next/23.16.8/24.2.0) | --- ### Release Notes <details> <summary>i18next/i18next (i18next)</summary> ### [`v24.2.0`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2420) [Compare Source](i18next/i18next@v24.1.2...v24.2.0) - feat(typescript): Add strictKeyChecks option to enforce checking key existence when defaultValue is used [2274](i18next/i18next#2274), fixes [2043](i18next/i18next#2043) ### [`v24.1.2`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2412) [Compare Source](i18next/i18next@v24.1.1...v24.1.2) - optimize fix: Bug Report: Unsafe Behavior in i18n.t Function Leading to Potential Code Execution [2273](i18next/i18next#2273) ### [`v24.1.1`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2411) [Compare Source](i18next/i18next@v24.1.0...v24.1.1) - fix: Bug Report: Unsafe Behavior in i18n.t Function Leading to Potential Code Execution [2273](i18next/i18next#2273) ### [`v24.1.0`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2410) [Compare Source](i18next/i18next@v24.0.5...v24.1.0) - try to address [2270](i18next/i18next#2270) by cloning the store data [2271](i18next/i18next#2271) ### [`v24.0.5`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2405) [Compare Source](i18next/i18next@v24.0.4...v24.0.5) - remove extra log for [2268](i18next/i18next#2268) ### [`v24.0.4`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2404) [Compare Source](i18next/i18next@v24.0.3...v24.0.4) - simplify fix: incorrect locale detected [2268](i18next/i18next#2268) ### [`v24.0.3`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2403) [Compare Source](i18next/i18next@v24.0.2...v24.0.3) - fix: incorrect locale detected [2268](i18next/i18next#2268) - fix: Intl.getCanonicalLocales throws with custom regions [2267](i18next/i18next#2267) ### [`v24.0.2`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2402) [Compare Source](i18next/i18next@v24.0.1...v24.0.2) - if no Intl api, log error and use dummy rule ### [`v24.0.1`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2401) [Compare Source](i18next/i18next@v24.0.0...v24.0.1) - early return and log error, if no Intl api ### [`v24.0.0`](https://github.com/i18next/i18next/blob/HEAD/CHANGELOG.md#2400) [Compare Source](i18next/i18next@v23.16.8...v24.0.0) **This is a major breaking release:** - remove support for older environments - remove old i18next JSON formats - To convert your existing v3 translations to the v4 format, have a look at [i18next-v4-format-converter](https://github.com/i18next/i18next-v4-format-converter) or this [web tool](https://i18next.github.io/i18next-v4-format-converter-web/). - remove support for compatibility to v1 API - Intl API is mandatory now and will not fallback anymore - possible compatibility layer for older formats: `test/compatibility/v4/v4Compatibility.js` - rename `initImmediate` to `initAsync` - fallback to `dev` language if plural rule not found - remove TypeScript v4 support. TypeScript v5 is now an optional peer dependency - addresses - [2244](i18next/i18next#2244) - [2184](i18next/i18next#2184) - [2213](i18next/i18next#2213) - [2206](i18next/i18next#2206) - [2208](i18next/i18next#2208) - [2148](i18next/i18next#2148) - [2254](i18next/i18next#2254) ➡️ check out the [migration guide](https://www.i18next.com/misc/migration-guide#v23.x.x-to-v24.0.0) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/384 Reviewed-by: Alexandre Soro <code@soro.dev> Co-authored-by: renovate <renovate@git.tristess.app> Co-committed-by: renovate <renovate@git.tristess.app>

perf: use Intl.getCanonicalLocale

80cda93

adrai added the breaking change label Oct 13, 2024

adrai added a commit that referenced this pull request Oct 13, 2024

use Intl.getCanonicalLocales function if available to format langua…

c374232

…ge code, like suggested in #2244

VIKTORVAV99 closed this Oct 28, 2024

VIKTORVAV99 deleted the replace_custom_language_formatting_code_with_Intl.getCanoncialLocales branch October 28, 2024 17:06

rodrigo3829l mentioned this pull request Nov 4, 2024

[Snyk] Upgrade i18next from 23.9.0 to 23.16.0 rodrigo3829l/db_api_mygarden_llc#18

Open

spdfalcon mentioned this pull request Nov 6, 2024

[Snyk] Upgrade i18next from 23.11.2 to 23.16.0 spdfalcon/company-2lang#16

Open

RMCampos mentioned this pull request Nov 7, 2024

[Snyk] Upgrade i18next from 23.15.2 to 23.16.0 ricardo-campos-org/react-typescript-todolist#148

Closed

carltrepanier mentioned this pull request Nov 7, 2024

[Snyk] Upgrade i18next from 23.15.2 to 23.16.0 carltrepanier/Portfolio#142

Merged

b23prodtm mentioned this pull request Nov 8, 2024

[Snyk] Upgrade i18next from 23.15.1 to 23.16.0 b23prodtm/passbolt_browser_extension#1

Open

snyk-io bot mentioned this pull request Nov 10, 2024

OPHJOD-851: Upgrade i18next from 23.15.2 to 23.16.1 Opetushallitus/jod-yksilo-ui#258

Closed

jeelsha mentioned this pull request Nov 11, 2024

[Snyk] Upgrade i18next from 23.7.8 to 23.16.2 jeelsha/proleven-frontend#3

Open

mbreadmachine mentioned this pull request Nov 12, 2024

[Snyk] Upgrade i18next from 23.15.1 to 23.16.2 mbreadmachine/arany-d-osztaly-weboldal#68

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: use Intl.getCanonicalLocale #2244

perf: use Intl.getCanonicalLocale #2244

VIKTORVAV99 commented Oct 13, 2024

coveralls commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

adrai commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

jamuhl commented Oct 13, 2024

adrai commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024 •

edited

Loading

adrai commented Oct 13, 2024 •

edited

Loading

VIKTORVAV99 commented Oct 13, 2024 •

edited

Loading

jamuhl commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

jamuhl commented Oct 14, 2024

perf: use Intl.getCanonicalLocale #2244

perf: use Intl.getCanonicalLocale #2244

Conversation

VIKTORVAV99 commented Oct 13, 2024

Checklist

coveralls commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

adrai commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

jamuhl commented Oct 13, 2024

adrai commented Oct 13, 2024

adrai commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024 • edited Loading

adrai commented Oct 13, 2024 • edited Loading

VIKTORVAV99 commented Oct 13, 2024 • edited Loading

jamuhl commented Oct 13, 2024

VIKTORVAV99 commented Oct 13, 2024

jamuhl commented Oct 14, 2024

VIKTORVAV99 commented Oct 13, 2024 •

edited

Loading

adrai commented Oct 13, 2024 •

edited

Loading

VIKTORVAV99 commented Oct 13, 2024 •

edited

Loading