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

Add support for nesting translations #368

Merged
merged 3 commits into from
Dec 4, 2020

Conversation

josephfarina
Copy link
Contributor

Addresses #365

@josephfarina josephfarina mentioned this pull request Dec 3, 2020
@aralroca
Copy link
Owner

aralroca commented Dec 3, 2020

So fast! 🔥

@aralroca aralroca self-requested a review December 3, 2020 20:21
if (getDicValue(dic, nestedKey) !== undefined) {
return nestedKey
}

const numKey = `${key}_${query.count}`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that it would make sense to also allow this nested format?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm do you mean something like this?

{
  "cart-message": {
     "cart-message_0": "",
     ...
}

If so, I think it would actually already work — i'm not sure how useful it is though — because this is only checking if the child key has one of the plural rules as a direct child. And if it does not, then the old rules will work the same.

Copy link
Owner

@aralroca aralroca Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking with:

{
  "cart-message": {
     "0": "",
     ...
}

Although the supported plurals are only 6, we also support an exact match, so perhaps it is also interesting to allow nesting for these "exact match" numbers.

{
  "cart-message": {
     "42": "Exact match when {{count}} === 42",
     "one": "...", 
     "other": ".."
  } 
}

I'm not sure how useful it would be, that's why I'm also asking. 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay I got you now! Yeah so for example Smartling, the reason nested support is needed, only supports one and other it doesn't even support zero let alone the 3 others. So I am not sure how useful it would be...I am more then happy to do it though if you think we should.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Thanks for your time 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aralroca just to confirm, should I add this feature or not? 😄

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes @josephfarina, it would be great! 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done @aralroca!

Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you put in the documentation an example with both ways?

{
  "cart-message_0": "The cart is empty", // when count === 0
  "cart-message_one": "The cart has only {{count}} product", // singular
  "cart-message_other": "The cart has {{count}} products", // plural
  "cart-message_999": "The cart is full", // when count === 999
}

or

{
  "cart-message": {
     "0": "The cart is empty", // when count === 0
     "one": "The cart has only {{count}} product", // singular
     "other": "The cart has {{count}} products", // plural
     "999": "The cart is full", // when count === 999
  } 
}

@aralroca aralroca added this to the 1.0.0 milestone Dec 3, 2020
Copy link
Owner

@aralroca aralroca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this another amazing contribution 👏 😊

@aralroca aralroca merged commit 1dd75f7 into aralroca:canary Dec 4, 2020
aralroca added a commit that referenced this pull request Dec 9, 2020
* Fix plurals (#347)

* Add plurals: zero, one, two, few, many, other

* Update node support

* Update node support

* Update node support

* Fix withTranslation type (#354)

* Replace CLI to next plugin + move project to typescript + refactor (#359)

* [WIP] Implement webpack loader

* Update .gitignore

* Implement loader when the _app.js has a getInitialProps

* Add missing pages on example

* Remove builder tests right now

* Fix default loadLocaleFrom on _app.js

* Implement src/_loader/hasGetInitialPropsOnAppJs.js

* add I18nProvider on top of _app.js if _app.js exist

* Load default app.js with I18nProvider

* Remove all deprecations from 0.19

* Implement loading namespaces from getInitialProps on pages

* Refactor

* Refactor loader

* Add tests + improve templateWithHoc

* Update dependences

* Implement basic template with loader

* Fix case of getServerSideProps

* Fix no export default

* Add loadNamespaces + refactor + add snapshots

* Add test cases + fix cases for getStaticProps and getServerSideProps

* Add more test cases + fix more cases about global exports

* Fix appWithI18n

* Update .npmignore

* [WIP] migrate to TypeScript

* Fix loader

* Apply fixes on index.js

* Fix tests

* add experimental version

* Implement logBuild

* Update package version to 1.0.0-experimental.6

* Add esModuleInterop=true

* Update tsconfig.json

* Add warnings

* Do dynamic prop from DynamicNamespace transparent

* Implement fallbacks (#333)

* Add fallback typescript type (#334)

* Add fallback typescript type

* Update index.d.ts

* Update index.d.ts

* Implement default namespace on useTranslation hook (#335)

* Improve type

* Fix Trans type

* Update dependencies

* Support components trans as object (#336)

* Support Trans 'components' property to be an object

* Fix docs

* Fix when _app.js is missing

* Change way to load webpack loader

* Modify todo list

* Remove next/app from appWithI18n + refactor

* Update docs

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update plurals docs

* Add migration guide `0.x` to `1.0.0` (#349)

* Add migration guide `0.x` to `1.0.0`

* Fix plural section

* Add yarn command

* docs: add dislick as a contributor (#350)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Improve docs

* Update README.md

* Fix src/pages in experimiental 1.0 (#352)

* Update examples + some fixes

* Improve example

* Improve examples

* Add warning about getInitialProps on pages

* add notes about appWithI18n on the migration guide

* Add useful links to the migration guide

* Fix imports from webpack loader in windows (#355)

* Fix imports from webpack loader in windows

* Fix clean helper on tests

* Fix clean helper on tests

* Fix comma

* Fix test

* Use webpack alias for absolute imports

* Move plugin files to typescript

* Update package version

Co-authored-by: Patrick <muff.pa@gmail.com>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Update README with the new examples (#360)

* Update README with the new examples

* Add quotes

* fix: compiled page pathname in windows (#362)

* fix: compiled page name in windows

* Update src/plugin/loader.ts

Co-authored-by: Aral Roca Gomez <aral-rg@hotmail.com>

Co-authored-by: Aral Roca Gomez <aral-rg@hotmail.com>

* docs: add vimutti77 as a contributor (#363)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Aral Roca Gomez <aral-rg@hotmail.com>

* Update package version

* Change config way i18provider (#369)

* Change way to use i18n config inside I18nProvider

* Add default

* Merge nested configs in nester I18nProviders (#370)

* Fix _app.js with class extended from NextApp (#371)

* Update CONTRIBUTING.md

* Clean build plugin packages (#372)

* Add support to customize the interpolation delimeter (#373)

* Add support for nesting translations (#368)

* Add support for nesting translations

* Add a nested version of translations example

* Add support for numbers in the nested keys

* docs: add josephfarina as a contributor (#374)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Aral Roca Gomez <aral-rg@hotmail.com>

* Update README.md

* Update README.md

* Update package version

* Update README

* Update README.md

* Update README.md

* Update example adding custom interpolation (#375)

* Re-add Translate type (#376)

* Improve interface Translate (#377)

The interface "Translate" describes the "translate" function in three variants.

In the following the variants of the return values of the function are considered.

1. the return value corresponds to a string or an object (of any complexity) - string | object

In the case of an object, the return type (string | object) requires a type cast from the function's user

Example 1
const translatedObject: TypeOfTranslatedObject = t(
translationKey,
    {},
    {returnObjects: true}
);

example 2
const translatedObject = t(
    translationKey,
    {},
    {returnObjects: true}
) as TypeOfTranslatedObject;

To make the use of the function more intuitive, the object type of the object to be translated is passed to the function call as generic type parameter.

Example 1 - Return of a simple object of the type TypeOfTranslatedObject

const translatedObject = t<TypeOfTranslatedObject>(
    translationKey,
    {},
    {returnObjects: true}
);

Example 2 - Return of an array whose contents correspond to the object type TypeOfTranslatedObject

const translatedObject = t<Array<TypeOfTranslatedObject>>(
    translationKey,
    {},
    {returnObjects: true}
);

The change made in the commit,
- introduces a generic type parameter and its initialization to a string,
- the type parameter is optional. If no type parameter is specified by the caller, the default value string is used.

The return value of the second and third variant corresponds to a string only and no other type.

The change made in the commit
- removes the generic type declaration, there is only one return type,
- an override from outside is not possible.

Co-authored-by: gurkerl83 <markus_gritsch@gmx.de>

* docs: add gurkerl83 as a contributor (#378)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Aral Roca Gomez <aral-rg@hotmail.com>

* Add bundle analyzer to the complex example (#379)

* Fix package (#380)

* Fix package

* Move types to the root

* Add (getT) to get the t function outside components / pages (#383)

* Support to consume t function outside React

* fix typo

* Fix issue with Node 10 (#386)

* Update README.md

* Update package version

Co-authored-by: Patrick <muff.pa@gmail.com>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Vantroy <vimutti@outlook.com>
Co-authored-by: Joey <Jrf61194@gmail.com>
Co-authored-by: gurkerl83 <gurkerl83@googlemail.com>
Co-authored-by: gurkerl83 <markus_gritsch@gmx.de>
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

Successfully merging this pull request may close these issues.

2 participants