YuriyKrasilnikov commented on code in PR #37790:
URL: https://github.com/apache/superset/pull/37790#discussion_r2783786242
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -569,6 +585,109 @@ const FiltersConfigForm = (
[filterId, form, formChanged],
);
+ const localizationEnabled = isFeatureEnabled(
+ FeatureFlag.EnableContentLocalization,
+ );
+
+ const currentTranslations: Translations =
+ formFilter?.translations ?? filterToEdit?.translations ?? {};
+
+ useEffect(() => {
+ if (filterToEdit?.translations) {
+ setNativeFilterFieldValues(form, filterId, {
+ translations: filterToEdit.translations,
+ });
+ }
+ }, [filterId, filterToEdit?.translations, form]);
+
+ useEffect(() => {
+ if (localizationEnabled) {
+ SupersetClient.get({
+ endpoint: '/api/v1/localization/available_locales',
+ }).then(
+ response => {
+ const { locales, default_locale } = response.json.result;
+ setAllLocales(locales);
+ setDefaultLocale(default_locale);
+ },
+ err => logging.error('Failed to fetch available locales', err),
+ );
+ }
+ }, [localizationEnabled]);
+
+ // Refs for values read during initialization but not triggering re-runs.
+ // Without refs, editing a translation would reset the active locale
+ // because currentTranslations changes on every keystroke.
+ const currentTranslationsRef = useRef(currentTranslations);
+ currentTranslationsRef.current = currentTranslations;
+ const userLocaleRef = useRef(userLocale);
+ userLocaleRef.current = userLocale;
+
+ // Initialize nameActiveLocale when switching between filters.
+ useEffect(() => {
+ if (
+ localizationEnabled &&
+ currentTranslationsRef.current.name?.[userLocaleRef.current]
+ ) {
+ setNameActiveLocale(userLocaleRef.current);
+ } else {
+ setNameActiveLocale(DEFAULT_LOCALE_KEY);
+ }
+ }, [localizationEnabled, filterId]);
+
+ const handleNameTranslationChange = useCallback(
+ (value: string) => {
+ const updated: Translations = {
+ ...currentTranslationsRef.current,
+ name: { ...currentTranslationsRef.current.name, [nameActiveLocale]:
value },
Review Comment:
Not a bug. Object spread with `undefined` is explicitly safe per ECMAScript
spec.
**CopyDataProperties** (ECMA-262, ยง14.18) step 3:
> If source is **undefined** or **null**, return target.
Runtime proof (Node v20):
```js
const translations = {};
({ ...translations.name, de: "German" }) // โ { de: "German" }
```
This is fundamentally different from **array spread** (`[...undefined]` โ
TypeError) because array spread requires an iterable, while object spread has
an explicit null/undefined guard in the spec.
The author was already aware โ line 630 in the same file uses optional
chaining for the *read* path
(`currentTranslationsRef.current.name?.[userLocaleRef.current]`), while
correctly relying on spread-is-safe behavior for the *construction* path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
