-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
!!!FEATURE: Reform i18n mechanism #3804
Conversation
33fb626
to
232e6a8
Compare
I like this, but wow it's also A LOT. Thanks for digging in! |
894dbeb
to
4d4bf35
Compare
Rebased to latest 9.0 state. |
Locally the e2e tests run fine:
|
4d4bf35
to
9b1bddb
Compare
9b1bddb
to
395502a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for refactoring the translation functionality.
When reading the code i really enjoyed finding common modern php patterns now reflected here in the code base like immutable value objects, collection of vo's, enums, explicit exceptions, etc (and also repositories) while on the other hand embracing functions when applicable like the main translation
function which is just that - a function - not a service, registry or anything which is quite how javascript was indented. I think this unique mix is a really good pattern for the future and this example i18n implementation gives a good boilerplate for future similar endpoints or functionality.
For a moment i was wondering if we had to respect the Neos.Neos.userInterface.scrambleTranslatedLabels
option but funnily this is sorely handled by the XliffService
and thus the rendered json will already be containing the #######
scrambles.
Soo much work and a well designed model :D Thanks!
packages/neos-ui-i18n/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lovely inline documentation thanks ❤️ !11!!!
teardownI18n(); | ||
}); | ||
|
||
it('loads the translation from the location specified in the current HTML document', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests 💯 :D
} | ||
} | ||
|
||
export class InvalidLocale extends Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like that you adapted the php code style regarding exceptions with factory methods like InvalidLocale.becauseOfInvalidIdentifier()
as proposed and used in the ESCR https://discuss.neos.io/t/rfc-updated-default-code-style/5836!!
export class Locale { | ||
private readonly intlPluralRules: Intl.PluralRules; | ||
|
||
private constructor( | ||
private readonly intlLocale: Intl.Locale, | ||
private readonly pluralRules: PluralRules | ||
) { | ||
this.intlPluralRules = new Intl.PluralRules(this.intlLocale.toString()); | ||
} | ||
|
||
public static create = (identifier: string, pluralRulesAsString: string): Locale => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is basically how we write value objects in php too !!! this is a great direction :D
Removes the underscore for clarity and modern style.
prelude to: #3773
refs: #3119
This PR reforms the UI's i18n mechanism by exposing a new function
translate
directly from the@neos-project/neos-ui-i18n
package.The function looks as follows:
And can be used like this:
The new mechanism is completely independent from registries. There's no longer a need for injecting the
I18nRegistry
to use translations, insteadtranslate
can be used directly.translate
will only work after the translations have been loaded during the bootstrapping process of@neos-project/neos-ui
. If it is used too early, it throws an exception.The function now also properly supports plural rules (via
Intl
Web API). ThaI18nRegistry
had only supported English plurals, but with broken rules (quantity=0 yielded singular instead of plural).Other changes included in this PR are:
@neos-project/neos-ui-i18n
is now TypeScript (savemanifest.js
)<I18n/>
-component was deprecatedI18nRegistry
is now properly documented, but was also deprecatedxliff.json
endpoint is now discovered via<link>
-tag, instead of initial data<link>
-tag is marked withprefetch
to slightly speed up the fetch request@neos-project/neos-ui-i18n
now has a comprehensive README.mdThis change is breaking, because translations may have relied on the broken plural rules. It is also breaking, because the signature for substitution parameters has changed slightly - although this should be non-breaking at runtime.