Skip to content

Commit

Permalink
Make Amsterdam an opt-in theme (elastic#212787)
Browse files Browse the repository at this point in the history
## Summary

This PR updates `DEFAULT_THEME_TAGS` used to determine what theme tags
are bundled in Kibana by default to only include the Borealis theme,
specifically `borealislight` and `borealisdark` theme tags. This change
is expected to decrease bundle sizes significantly and get back to
bundling a single theme, not two (4 → 2 theme tags).

Now that Serverless, `9.0`, and `main` all run with Borealis, there's no
risk in removing Amsterdam from the bundle and decreasing Kibana bundle
sizes.

We need to keep the feature flag in code for the time being to easily
test future Borealis iterations.

Amsterdam will still be available as an opt-in theme and is meant to be
used locally when testing changes to be backported to 8.x versions that
use Amsterdam. To do so, Kibana needs to be started/built with
`KBN_OPTIMIZER_THEMES` environment variable set and the feature flag
overridden in `kibana.dev.yml`.

```yml
# config/kibana.dev.yml
feature_flags.overrides.coreRendering.defaultThemeName: amsterdam
```

```shell
# Run dev server with both borealis and Amsterdam theme tags
KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start
```

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
tkajtoch and elasticmachine authored Mar 4, 2025
1 parent 14b18ac commit 4dd8de8
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ describe('getOptimizerCacheKey()', () => {
"reactVersion": "17",
"repoRoot": <absolute path>,
"themeTags": Array [
"v8light",
"v8dark",
"borealislight",
"borealisdark",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jest.doMock('./render_template', () => ({
}));

export const getThemeTagMock = jest.fn();
jest.doMock('./get_theme_tag', () => ({
jest.doMock('../theme', () => ({
getThemeTag: getThemeTagMock,
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,7 @@ describe('bootstrapRenderer', () => {

expect(getThemeTagMock).toHaveBeenLastCalledWith(
expect.objectContaining({
// 'amsterdam' is mapped to 'v8' for backwards compatibility
name: 'v8',
name: 'amsterdam',
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import {
import type { IUiSettingsClient } from '@kbn/core-ui-settings-server';
import type { UiPlugins } from '@kbn/core-plugins-base-server-internal';
import { InternalUserSettingsServiceSetup } from '@kbn/core-user-settings-server-internal';
import { getThemeTag } from '../theme';
import { getPluginsBundlePaths } from './get_plugin_bundle_paths';
import { getJsDependencyPaths } from './get_js_dependency_paths';
import { getThemeTag } from './get_theme_tag';
import { renderTemplate } from './render_template';
import { getBundlesHref } from '../render_utils';

Expand Down Expand Up @@ -89,7 +89,7 @@ export const bootstrapRendererFactory: BootstrapRendererFactory = ({
}

const themeTag = getThemeTag({
name: !themeName || themeName === 'amsterdam' ? 'v8' : themeName,
name: themeName,
darkMode,
});
const bundlesHref = getBundlesHref(baseHref);
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ jest.doMock('./get_apm_config', () => {
getApmConfig: getApmConfigMock,
};
});

export const getIsThemeBundledMock = jest.fn();
jest.doMock('./theme', () => {
return {
isThemeBundled: getIsThemeBundledMock,
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getScriptPathsMock,
getBrowserLoggingConfigMock,
getApmConfigMock,
getIsThemeBundledMock,
} from './rendering_service.test.mocks';

import { load } from 'cheerio';
Expand Down Expand Up @@ -615,12 +616,41 @@ describe('RenderingService', () => {
globalClient: uiSettingsServiceMock.createClient(),
};

getIsThemeBundledMock.mockImplementation((name) => ['borealis', 'amsterdam'].includes(name));

let renderResult = await render(createKibanaRequest(), uiSettings);
expect(getIsThemeBundledMock).toHaveBeenCalledWith('borealis');
expect(renderResult).toContain(',&quot;name&quot;:&quot;borealis&quot;');

themeName$.next('amsterdam');
renderResult = await render(createKibanaRequest(), uiSettings);
expect(renderResult).toContain(',&quot;name&quot;:&quot;amsterdam&quot;');
});

it('falls back to the default theme if theme is not bundled', async () => {
// setup and render added to assert the current theme name
const { render } = await service.setup(mockRenderingSetupDeps);
const themeName$ = new BehaviorSubject<ThemeName>('unknown' as any);
const getStringValue$ = jest
.fn()
.mockImplementation((_, fallback) => themeName$.asObservable());
service.start({
...mockRenderingStartDeps,
featureFlags: {
...mockRenderingStartDeps.featureFlags,
getStringValue$,
},
});

const uiSettings = {
client: uiSettingsServiceMock.createClient(),
globalClient: uiSettingsServiceMock.createClient(),
};

getIsThemeBundledMock.mockReturnValue(false);

const renderResult = await render(createKibanaRequest(), uiSettings);
expect(renderResult).toContain(',&quot;name&quot;:&quot;borealis&quot;');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
import { filterUiPlugins } from './filter_ui_plugins';
import { getApmConfig } from './get_apm_config';
import type { InternalRenderingRequestHandlerContext } from './internal_types';
import { isThemeBundled } from './theme';

type RenderOptions =
| RenderingSetupDeps
Expand Down Expand Up @@ -134,7 +135,16 @@ export class RenderingService {
featureFlags
.getStringValue$<ThemeName>(DEFAULT_THEME_NAME_FEATURE_FLAG, DEFAULT_THEME_NAME)
// Parse the input feature flag value to ensure it's of type ThemeName
.pipe(map((value) => parseThemeNameValue(value)))
// and that it's bundled with this build of Kibana
.pipe(
map((themeName) => {
if (isThemeBundled(themeName)) {
return parseThemeNameValue(themeName);
}

return DEFAULT_THEME_NAME;
})
)
.subscribe(this.themeName$);
}

Expand Down
82 changes: 82 additions & 0 deletions src/core/packages/rendering/server-internal/src/theme.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { getThemeTag, isThemeBundled } from './theme';

describe('getThemeTag', () => {
it('returns the correct value for name:amsterdam and darkMode:false', () => {
expect(
getThemeTag({
name: 'v8',
darkMode: false,
})
).toEqual('v8light');
});

it('returns the correct value for name:amsterdam and darkMode:true', () => {
expect(
getThemeTag({
name: 'v8',
darkMode: true,
})
).toEqual('v8dark');
});

it('returns the correct value for other other theme names and darkMode:false', () => {
expect(
getThemeTag({
name: 'borealis',
darkMode: false,
})
).toEqual('borealislight');
});

it('returns the correct value for other other theme names and darkMode:true', () => {
expect(
getThemeTag({
name: 'borealis',
darkMode: true,
})
).toEqual('borealisdark');
});
});

describe('isThemeBundled', () => {
let originalKbnOptimizerThemes: any;

beforeAll(() => {
originalKbnOptimizerThemes = process.env.KBN_OPTIMIZER_THEMES;
});

afterAll(() => {
process.env.KBN_OPTIMIZER_THEMES = originalKbnOptimizerThemes;
});

it('returns true when both light and dark mode theme tags are included in KBN_OPTIMIZER_THEMES', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v8light,v8dark,borealislight,borealisdark';
expect(isThemeBundled('amsterdam')).toEqual(true);
expect(isThemeBundled('borealis')).toEqual(true);
});

it('returns false when either theme tag is missing in KBN_OPTIMIZER_THEMES for given theme name', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v8light,borealisdark,borealisdark';
expect(isThemeBundled('amsterdam')).toEqual(false);
expect(isThemeBundled('borealis')).toEqual(false);
});

it('uses default themes when KBN_OPTIMIZER_THEMES is not set', () => {
delete process.env.KBN_OPTIMIZER_THEMES;
expect(isThemeBundled('borealis')).toEqual(true);
expect(isThemeBundled('sometheme' as any)).toEqual(false);

process.env.KBN_OPTIMIZER_THEMES = '';
expect(isThemeBundled('borealis')).toEqual(true);
expect(isThemeBundled('sometheme' as any)).toEqual(false);
});
});
34 changes: 34 additions & 0 deletions src/core/packages/rendering/server-internal/src/theme.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { parseThemeTags, ThemeName, ThemeTag } from '@kbn/core-ui-settings-common';

export const getThemeTag = ({ name, darkMode }: { name: string; darkMode: boolean }) => {
// Amsterdam theme is called `v8` internally
// and should be kept this way for compatibility reasons.
if (name === 'amsterdam') {
name = 'v8';
}

return `${name}${darkMode ? 'dark' : 'light'}`;
};

/**
* Check whether the theme is bundled in the current kibana build.
* For a theme to be considered bundled both light and dark mode
* styles must be included.
*/
export const isThemeBundled = (name: ThemeName): boolean => {
const bundledThemeTags = parseThemeTags(process.env.KBN_OPTIMIZER_THEMES);

return (
bundledThemeTags.includes(getThemeTag({ name, darkMode: false }) as ThemeTag) &&
bundledThemeTags.includes(getThemeTag({ name, darkMode: true }) as ThemeTag)
);
};
2 changes: 1 addition & 1 deletion src/core/packages/ui-settings/common/src/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type ThemeTags = readonly ThemeTag[];
* An array of theme tags available in Kibana by default when not customized
* using KBN_OPTIMIZER_THEMES environment variable.
*/
export const DEFAULT_THEME_TAGS: ThemeTags = SUPPORTED_THEME_TAGS;
export const DEFAULT_THEME_TAGS: ThemeTags = ThemeBorealisTags;

export const FALLBACK_THEME_TAG: ThemeTag = 'borealislight';

Expand Down

0 comments on commit 4dd8de8

Please sign in to comment.