Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 63 additions & 47 deletions superset-frontend/src/preamble.ts
Original file line number Diff line number Diff line change
@@ -1,83 +1,99 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { setConfig as setHotLoaderConfig } from 'react-hot-loader';
import dayjs from 'dayjs';
// eslint-disable-next-line no-restricted-imports
import { configure, makeApi, initFeatureFlags } from '@superset-ui/core';
import {
configure,
makeApi,
initFeatureFlags,
SupersetClient,
} from '@superset-ui/core';
import setupClient from './setup/setupClient';
import setupColors from './setup/setupColors';
import setupFormatters from './setup/setupFormatters';
import setupDashboardComponents from './setup/setupDashboardComponents';
import { User } from './types/bootstrapTypes';
import getBootstrapData, { applicationRoot } from './utils/getBootstrapData';

configure();

This comment was marked as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this for? Is there a loading happening between this and the language pack loading?

@mistercrunch mistercrunch Jul 14, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm unclear what exactly it does, but it prepares a bunch of things around the various "registries" and such. It logs a bunch of things to the console if called too late. Might be worth digging a bit deeper. I'll double check there's no important perf costs in calling it twice.


// Set hot reloader config
if (process.env.WEBPACK_MODE === 'development') {
setHotLoaderConfig({ logLevel: 'debug', trackTailUpdates: false });
}

// eslint-disable-next-line import/no-mutable-exports
// Grab initial bootstrap data
const bootstrapData = getBootstrapData();

// Configure translation
if (typeof window !== 'undefined') {
configure({ languagePack: bootstrapData.common.language_pack });
dayjs.locale(bootstrapData.common.locale);
} else {
configure();
}

// Configure feature flags
initFeatureFlags(bootstrapData.common.feature_flags);

// Setup SupersetClient
// Setup SupersetClient early so we can fetch language pack
setupClient({ appRoot: applicationRoot() });

setupColors(
bootstrapData.common.extra_categorical_color_schemes,
bootstrapData.common.extra_sequential_color_schemes,
);
// Load language pack before anything else
(async () => {
const lang = bootstrapData.common.locale || 'en';
if (lang !== 'en') {
try {
const { json: languagePack } = await SupersetClient.get({
endpoint: `/superset/language_pack/${lang}/`,
});
// Second call to configure to set the language pack
configure({ languagePack });
dayjs.locale(lang);
} catch (err) {
console.warn(
'Failed to fetch language pack, falling back to default.',
err,
);
configure();
dayjs.locale('en');
}
}

// Setup number formatters
setupFormatters(
bootstrapData.common.d3_format,
bootstrapData.common.d3_time_format,
);
// Continue with rest of setup
initFeatureFlags(bootstrapData.common.feature_flags);

setupDashboardComponents();
setupColors(
bootstrapData.common.extra_categorical_color_schemes,
bootstrapData.common.extra_sequential_color_schemes,
);

const getMe = makeApi<void, User>({
method: 'GET',
endpoint: '/api/v1/me/',
});
setupFormatters(
bootstrapData.common.d3_format,
bootstrapData.common.d3_time_format,
);

/**
* When you re-open the window, we check if you are still logged in.
* If your session expired or you signed out, we'll redirect to login.
* If you aren't logged in in the first place (!isActive), then we shouldn't do this.
*/
if (bootstrapData.user?.isActive) {
document.addEventListener('visibilitychange', () => {
// we only care about the tab becoming visible, not vice versa
if (document.visibilityState !== 'visible') return;
setupDashboardComponents();

getMe().catch(() => {
// ignore error, SupersetClient will redirect to login on a 401
});
const getMe = makeApi<void, User>({
method: 'GET',
endpoint: '/api/v1/me/',
});
}

if (bootstrapData.user?.isActive) {
document.addEventListener('visibilitychange', () => {
if (document.visibilityState === 'visible') {
getMe().catch(() => {
// SupersetClient will redirect to login on 401
});
}
});
}
})();
2 changes: 1 addition & 1 deletion superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def _try_json_readsha(filepath: str, length: int) -> str | None:
}
# Turning off i18n by default as translation in most languages are
# incomplete and not well maintained.
LANGUAGES = {}
# LANGUAGES = {}


# Override the default d3 locale format
Expand Down
2 changes: 0 additions & 2 deletions superset/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
from superset.extensions import cache_manager
from superset.reports.models import ReportRecipientType
from superset.superset_typing import FlaskResponse
from superset.translations.utils import get_language_pack
from superset.utils import core as utils, json
from superset.utils.filters import get_dataset_access_filters
from superset.views.error_handling import json_error_response
Expand Down Expand Up @@ -364,7 +363,6 @@ def cached_common_bootstrap_data( # pylint: disable=unused-argument
"static_assets_prefix": conf["STATIC_ASSETS_PREFIX"],
"conf": frontend_config,
"locale": language,
"language_pack": get_language_pack(language),
"d3_format": conf.get("D3_FORMAT"),
"d3_time_format": conf.get("D3_TIME_FORMAT"),
"currencies": conf.get("CURRENCIES"),
Expand Down
20 changes: 19 additions & 1 deletion superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@

import contextlib
import logging
import os
from datetime import datetime
from typing import Any, Callable, cast
from urllib import parse

from flask import abort, flash, g, redirect, request, Response, url_for
from flask import abort, flash, g, redirect, request, Response, send_file, url_for
from flask_appbuilder import expose
from flask_appbuilder.security.decorators import (
has_access,
Expand Down Expand Up @@ -855,7 +856,7 @@
if original_params := request.query_string.decode():
url = f"{url}&{original_params}"
if hash_ := state.get("anchor", state.get("hash")):
url = f"{url}#{hash_}"

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.
return redirect(url)

@api
Expand Down Expand Up @@ -891,6 +892,23 @@
datasource.raise_for_access()
return json_success(json.dumps(sanitize_datasource_data(datasource.data)))

@event_logger.log_this
@expose("/language_pack/<lang>/")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mistercrunch It would be nice to put this endpoint under /api/v1 to be an official endpoint.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah I thought through this wondering if this is a private or public endpoint, and thought ideally should be public, but given the partitioning of libraries in the frontend, and the fact that our i18n framework is monolithic currently (no segmentation or context based on the provenance of the strings), it felt like it would require significantly more work to do this properly.

Not sure whether the extension framework SIP mentions i18n, but it may be a bit of a blindspot.

Without going to deep into it, it seems that if we want for extensions to be localized/localizable, we'd need some way to segment the language pack based on package provenance (backend/frontend, and break down per library on the frontend side). The whole i18n stack might need to offer different ways to fetch and apply translations.

Happy to help think through this, but it's a fairly significant amount of work.

Note that one minor and valuable thing I considered while doing the work here was to find a way to add metadata to translation to identify what is frontend/backend, and make sure the endpoint only returns the strings intended for the frontend ... The gettext-based/flask-babel framework we use does offer ways to do such things, but I encountered some limitations, and decided to punt on these.

This PR here addresses something pretty important - removing the language pack from bootstrap data - but there's much more to do in this area.

def language_pack(self, lang: str) -> FlaskResponse:
base_dir = os.path.join(
os.path.dirname(__file__),
"..",
"translations",
lang,
"LC_MESSAGES",
"messages.json",
)
if os.path.isfile(base_dir):
return send_file(base_dir, mimetype="application/json")
return json_error_response(
"Language pack doesn't exist on the server", status=404
)

@event_logger.log_this
@expose("/welcome/")
def welcome(self) -> FlaskResponse:
Expand Down
Loading