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

Fix standalone registeration #556

Merged
merged 2 commits into from
Jan 19, 2023
Merged

Fix standalone registeration #556

merged 2 commits into from
Jan 19, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jan 12, 2023

In 2.0.0 @nextcluod/l10n became independent from global OC for being standalone.
That is great and really useful for me.

But a translation registration requires global window._oc_l10n_registry_* objects to exist. Otherwise, registration fails with Cannot get property of undefined.

  • Added checking of _oc_l10n_registry_ objects existence and creating if they do not exist.
  • Added tests for translation registration

@ShGKme ShGKme added the bug Something isn't working label Jan 12, 2023
lib/registry.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

👍
Just one comment about the console warning

Registration of a translation bundle in standalone application didn't work 
without initializing global `_oc_l10n_registry_*` objects.

Add global registry initialization in `register` function.

Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
Comment on lines 68 to 79
export function getAppTranslations(appId: string): AppTranslations {
if (
typeof window._oc_l10n_registry_translations === 'undefined'
|| typeof window._oc_l10n_registry_plural_functions === 'undefined'
typeof window._oc_l10n_registry_translations?.[appId] === 'undefined'
|| typeof window._oc_l10n_registry_plural_functions?.[appId] === 'undefined'
) {
console.warn('No OC L10N registry found')
return {
translations: {},
pluralFunction: (number: number) => number,
}
console.warn(`No translation for appId "${appId}" have been registered`)
}

return {
translations: window._oc_l10n_registry_translations[appId] || {},
pluralFunction: window._oc_l10n_registry_plural_functions[appId],
translations: window._oc_l10n_registry_translations?.[appId] ?? {},
pluralFunction: window._oc_l10n_registry_plural_functions?.[appId] ?? ((number: number) => number),
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update warns logic here.

Since it doesn't matter if "no app translation has been registered" or "only appId hasn't been registered", check both situations with optional chaining.

Warn is always about concrete appId, as there is no "general translation registration." It is always some app translation registration.

Also, return default stub for both translations and pluralFunction if there is no value.

@skjnldsv skjnldsv merged commit 0fe2ab4 into master Jan 19, 2023
@skjnldsv skjnldsv deleted the fix-standalone-register branch January 19, 2023 11:30
@skjnldsv skjnldsv mentioned this pull request Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants