From ac3fc27a5397456630f974f84bee64f597500b55 Mon Sep 17 00:00:00 2001 From: Tomasz Kajtoch Date: Wed, 28 May 2025 15:41:19 +0200 Subject: [PATCH] Add conditional switching between EUI releases (#219818) ## Summary This PR simplifies the weekly EUI upgrade and backport process by conditionally aliasing `@elastic/eui` in shared deps webpack configurations. # Backstory The EUI team (@elastic/eui-team) is responsible for keeping EUI up to date in Kibana. Historically, this has been a relatively straightforward (yet time-consuming) process, however, due to `8.x` backport complexities caused by it using a different theme, it has become way more demanding on everybody involved. EUI is released on weekly basis. Each week, we release official EUI versions tagged `latest` in npmjs and get a PR open that updates the package in kibana `main`. Our upgrade PRs tend to require anywhere between 2 and 25 codeowner reviews due to the number of snapshots we need to update while working on the EUI upgrade PRs. These snapshot changes are 99% of the time harmless, yet it still takes 2+ full workdays to ping teams and get all reviews necessary to get the PR merged. Generally speaking, we aim to have the upgrade PR open on Monday and merged by Friday. ## The issue with `8.x` backports Kibana 8.x uses the Amsterdam theme instead of Borealis, which is used in Kibana 9.0 and up. To keep 8.x up to date, for each official EUI release we prepare another special Kibana 8.x only release of EUI (e.g., `101.2.0-amsterdam.0`). These special releases have the theme hardcoded to Amsterdam at compile-time to avoid any initial theme errors Kibana could otherwise experience. This is done primarily because some areas in Kibana read EUI theme values outside of React components, and we have no stable way to determine what the active theme is since there's no context information. This is where we need to fall back to Amsterdam in 8.x and Borealis in 9.x. **Since there are two different EUI versions - one for Kibana `main` and 9.0, and another for 8.x branches, we cannot use the automated backports feature**. Instead, we open two separate PRs and configure backport labels accordingly. Having two PRs is far from ideal since codeowners need to review our changes twice, and we're more likely to make mistakes. # Our proposal Following the recently introduced React version switching logic, we want to conditionally switch between two `@elastic/eui` releases depending on the kibana branch/version while keeping automated backports possible. To achieve that, I added a dependency alias `@elastic/eui-amsterdam` that points to the Amsterdam EUI release and configured `resolve.alias` in shared deps to resolve the correct dependency based on the optional `EUI_AMSTERDAM` environment variable. When this change is merged to `main` and backported to `9.0` and `8.19`, I'll open a follow-up PR to the `8.19` branch updating the default value of `EUI_AMSTERDAM` to `"true"`. This should result in no conflicts and be easy to follow. Since 8.19 [uses the Amsterdam release of `@elastic/eui`](https://github.com/elastic/kibana/blob/8.19/package.json#L126) (e.g., `101.2.0-amsterdam.0`), there's no risk backporting this PR as-is without `EUI_AMSTERDAM` configured beforehand. ## What does it change? With this setup, we'll be able to update versions of `@elastic/eui` and `@elastic/eui-amsterdam` at the same time in a single PR and make use of automated kibana backports. There will be only one set of changes to review by codeowners, and if there are any failing tests when backporting to `8.19` due to, for example, changed color values, we can follow the regular kibana procedures and fix them right in the created backport PR. It'll simplify our workflow quite drastically while keeping the same level of quality. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../commands/bootstrap/bootstrap_command.mjs | 3 +- kbn_pm/src/commands/watch_command.mjs | 1 + kbn_pm/src/lib/bazel.mjs | 21 +++++++--- package.json | 1 + packages/kbn-dependency-ownership/src/rule.ts | 1 + src/dev/license_checker/config.ts | 3 ++ .../kbn-ui-shared-deps-npm/BUILD.bazel | 2 + .../kbn-ui-shared-deps-npm/webpack.config.js | 18 +++++++- .../kbn-ui-shared-deps-src/BUILD.bazel | 3 +- .../kbn-ui-shared-deps-src/webpack.config.js | 21 ++++++++-- yarn.lock | 41 +++++++++++++++++++ 11 files changed, 103 insertions(+), 12 deletions(-) diff --git a/kbn_pm/src/commands/bootstrap/bootstrap_command.mjs b/kbn_pm/src/commands/bootstrap/bootstrap_command.mjs index 4983257bba36..d1929fb5d032 100644 --- a/kbn_pm/src/commands/bootstrap/bootstrap_command.mjs +++ b/kbn_pm/src/commands/bootstrap/bootstrap_command.mjs @@ -55,6 +55,7 @@ export const command = { const validate = args.getBooleanValue('validate') ?? true; const quiet = args.getBooleanValue('quiet') ?? false; const reactVersion = process.env.REACT_18 ? '18' : '17'; + const euiAmsterdam = process.env.EUI_AMSTERDAM === 'true'; const vscodeConfig = args.getBooleanValue('vscode') ?? (process.env.KBN_BOOTSTRAP_NO_VSCODE ? false : true); @@ -115,7 +116,7 @@ export const command = { } await time('pre-build webpack bundles for packages', async () => { - await Bazel.buildWebpackBundles(log, { offline, quiet, reactVersion }); + await Bazel.buildWebpackBundles(log, { offline, quiet, reactVersion, euiAmsterdam }); }); await Promise.all([ diff --git a/kbn_pm/src/commands/watch_command.mjs b/kbn_pm/src/commands/watch_command.mjs index 9a98b5c12810..4932bb07eda0 100644 --- a/kbn_pm/src/commands/watch_command.mjs +++ b/kbn_pm/src/commands/watch_command.mjs @@ -28,6 +28,7 @@ export const command = { await Bazel.watch(log, { offline: args.getBooleanValue('offline') ?? true, reactVersion: process.env.REACT_18 ? '18' : '17', + euiAmsterdam: process.env.EUI_AMSTERDAM === 'true', }); }, }; diff --git a/kbn_pm/src/lib/bazel.mjs b/kbn_pm/src/lib/bazel.mjs index b4203dda0751..0c664beab446 100644 --- a/kbn_pm/src/lib/bazel.mjs +++ b/kbn_pm/src/lib/bazel.mjs @@ -58,7 +58,7 @@ function throwBazelError(log, name, code, output) { /** * @param {import('./log.mjs').Log} log * @param {string[]} inputArgs - * @param {{ quiet?: boolean; offline?: boolean, reactVersion?: string, env?: Record } | undefined} opts + * @param {{ quiet?: boolean; offline?: boolean, reactVersion?: string, euiAmsterdam?: boolean, env?: Record } | undefined} opts */ async function runBazel(log, inputArgs, opts = undefined) { const bazel = (await getBazelRunner()).runBazel; @@ -66,11 +66,16 @@ async function runBazel(log, inputArgs, opts = undefined) { const args = [ ...inputArgs, `--define=REACT_18=${opts?.reactVersion === '18' ? 'true' : 'false'}`, + `--define=EUI_AMSTERDAM=${opts?.euiAmsterdam ? 'true' : 'false'}`, ...(opts?.offline ? ['--config=offline'] : []), ]; log.debug(`> bazel ${args.join(' ')}`); await bazel(args, { - env: { ...opts?.env, REACT_18: opts?.reactVersion === '18' ? 'true' : 'false' }, + env: { + ...opts?.env, + REACT_18: opts?.reactVersion === '18' ? 'true' : 'false', + EUI_AMSTERDAM: opts?.euiAmsterdam ? 'true' : 'false', + }, cwd: REPO_ROOT, quiet: opts?.quiet, logPrefix: Color.info('[bazel]'), @@ -83,7 +88,7 @@ async function runBazel(log, inputArgs, opts = undefined) { /** * * @param {import('./log.mjs').Log} log - * @param {{ offline: boolean, reactVersion?: string } | undefined} opts + * @param {{ offline: boolean, reactVersion?: string, euiAmsterdam?: boolean } | undefined} opts */ export async function watch(log, opts = undefined) { const ibazel = (await getBazelRunner()).runIBazel; @@ -98,11 +103,16 @@ export async function watch(log, opts = undefined) { '--show_result=1', ...(opts?.offline ? ['--config=offline'] : []), `--define=REACT_18=${opts?.reactVersion === '18' ? 'true' : 'false'}`, + `--define=EUI_AMSTERDAM=${opts?.euiAmsterdam ? 'true' : 'false'}`, ]; log.debug(`> ibazel ${args.join(' ')}`); await ibazel(args, { cwd: REPO_ROOT, - env: { ...process.env, REACT_18: opts?.reactVersion === '18' ? 'true' : 'false' }, + env: { + ...process.env, + REACT_18: opts?.reactVersion === '18' ? 'true' : 'false', + EUI_AMSTERDAM: opts?.euiAmsterdam ? 'true' : 'false', + }, logPrefix: Color.info('[ibazel]'), onErrorExit(code, output) { throwBazelError(log, 'ibazel', code, output); @@ -167,13 +177,14 @@ export async function installYarnDeps(log, opts = undefined) { /** * @param {import('./log.mjs').Log} log - * @param {{ offline?: boolean, quiet?: boolean, reactVersion?: string } | undefined} opts + * @param {{ offline?: boolean, quiet?: boolean, reactVersion?: string, euiAmsterdam?: boolean } | undefined} opts */ export async function buildWebpackBundles(log, opts = undefined) { await runBazel(log, ['build', ...BAZEL_TARGETS, '--show_result=1'], { offline: opts?.offline, quiet: opts?.quiet, reactVersion: opts?.reactVersion, + euiAmsterdam: opts?.euiAmsterdam, }); log.success('shared bundles built'); diff --git a/package.json b/package.json index d6f4075b6e0e..3271f4334a27 100644 --- a/package.json +++ b/package.json @@ -127,6 +127,7 @@ "@elastic/elasticsearch": "9.0.2", "@elastic/ems-client": "8.6.3", "@elastic/eui": "102.2.0", + "@elastic/eui-amsterdam": "npm:@elastic/eui@102.2.0-amsterdam.0", "@elastic/eui-theme-borealis": "1.0.0", "@elastic/filesaver": "1.1.2", "@elastic/monaco-esql": "^3.1.1", diff --git a/packages/kbn-dependency-ownership/src/rule.ts b/packages/kbn-dependency-ownership/src/rule.ts index b884891d68cc..1348c1f038f4 100644 --- a/packages/kbn-dependency-ownership/src/rule.ts +++ b/packages/kbn-dependency-ownership/src/rule.ts @@ -41,6 +41,7 @@ export function packageFilter(pkg: string) { !pkg.startsWith('@kbn/') && // The EUI team owns the EUI packages, and are not covered by renovate pkg !== '@elastic/eui' && + pkg !== '@elastic/eui-amsterdam' && pkg !== '@elastic/eui-theme-borealis' && // Operations owns node, and is not covered by renovate pkg !== '@types/node' diff --git a/src/dev/license_checker/config.ts b/src/dev/license_checker/config.ts index 592cc6e875cd..77604b694623 100644 --- a/src/dev/license_checker/config.ts +++ b/src/dev/license_checker/config.ts @@ -89,6 +89,9 @@ export const LICENSE_OVERRIDES = { '@mapbox/jsonlint-lines-primitives@2.0.2': ['MIT'], // license in readme https://github.com/tmcw/jsonlint '@elastic/ems-client@8.6.3': ['Elastic License 2.0'], '@elastic/eui@102.2.0': ['Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0'], + '@elastic/eui-amsterdam@102.2.0-amsterdam.0': [ + 'Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0', + ], '@elastic/eui-theme-borealis@1.0.0': ['Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0'], 'language-subtag-registry@0.3.21': ['CC-BY-4.0'], // retired ODC‑By license https://github.com/mattcg/language-subtag-registry 'buffers@0.1.1': ['MIT'], // license in importing module https://www.npmjs.com/package/binary diff --git a/src/platform/packages/private/kbn-ui-shared-deps-npm/BUILD.bazel b/src/platform/packages/private/kbn-ui-shared-deps-npm/BUILD.bazel index 44f63e35acac..586ac6b7eee3 100644 --- a/src/platform/packages/private/kbn-ui-shared-deps-npm/BUILD.bazel +++ b/src/platform/packages/private/kbn-ui-shared-deps-npm/BUILD.bazel @@ -40,6 +40,7 @@ RUNTIME_DEPS = [ "@npm//@elastic/apm-rum-core", "@npm//@elastic/charts", "@npm//@elastic/eui", + "@npm//@elastic/eui-amsterdam", "@npm//@elastic/numeral", "@npm//@emotion/cache", "@npm//@emotion/react", @@ -98,6 +99,7 @@ webpack_cli( }, "//conditions:default": { "NODE_ENV": "development", + "EUI_AMSTERDAM": "$(EUI_AMSTERDAM)", }, }) ) diff --git a/src/platform/packages/private/kbn-ui-shared-deps-npm/webpack.config.js b/src/platform/packages/private/kbn-ui-shared-deps-npm/webpack.config.js index 15b9a1ae417b..6b1cbdc7f701 100644 --- a/src/platform/packages/private/kbn-ui-shared-deps-npm/webpack.config.js +++ b/src/platform/packages/private/kbn-ui-shared-deps-npm/webpack.config.js @@ -20,6 +20,8 @@ const WEBPACK_SRC = require.resolve('webpack'); const REPO_ROOT = Path.resolve(__dirname, '..', '..', '..', '..', '..'); +const useEuiAmsterdamRelease = process.env.EUI_AMSTERDAM === 'true'; + /** @returns {import('webpack').Configuration} */ module.exports = (_, argv) => { const outputPath = argv.outputPath ? Path.resolve(argv.outputPath) : UiSharedDepsNpm.distDir; @@ -138,7 +140,21 @@ module.exports = (_, argv) => { resolve: { alias: { - '@elastic/eui$': '@elastic/eui/optimize/es', + // @elastic/eui-amsterdam is a package alias defined in Kibana's package.json + // that points to special EUI releases bundled with Amsterdam set as the default theme + // and meant to be used with Kibana 8.x. Kibana 9.0 and later use the Borealis theme + // and should import from the regular @elastic/eui package. + // TODO: Remove when Kibana 8.19 is EOL and Amsterdam backports aren't needed anymore + // https://github.com/elastic/kibana/issues/221593 + '@elastic/eui$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es' + : '@elastic/eui/optimize/es', + '@elastic/eui/optimize/es/components/provider/nested$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es/components/provider/nested' + : '@elastic/eui/optimize/es/components/provider/nested', + '@elastic/eui/optimize/es/services/theme/warning$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es/services/theme/warning' + : '@elastic/eui/optimize/es/services/theme/warning', moment: MOMENT_SRC, // NOTE: Used to include react profiling on bundles // https://gist.github.com/bvaughn/25e6233aeb1b4f0cdb8d8366e54a3977#webpack-4 diff --git a/src/platform/packages/private/kbn-ui-shared-deps-src/BUILD.bazel b/src/platform/packages/private/kbn-ui-shared-deps-src/BUILD.bazel index b8997e635f87..63e2ce08634b 100644 --- a/src/platform/packages/private/kbn-ui-shared-deps-src/BUILD.bazel +++ b/src/platform/packages/private/kbn-ui-shared-deps-src/BUILD.bazel @@ -59,7 +59,8 @@ webpack_cli( "NODE_ENV": "production", }, "//conditions:default": { - "NODE_ENV": "development" + "NODE_ENV": "development", + "EUI_AMSTERDAM": "$(EUI_AMSTERDAM)", }, }), visibility = ["//visibility:public"], diff --git a/src/platform/packages/private/kbn-ui-shared-deps-src/webpack.config.js b/src/platform/packages/private/kbn-ui-shared-deps-src/webpack.config.js index 5060b675c6ae..5c03078c8f4e 100644 --- a/src/platform/packages/private/kbn-ui-shared-deps-src/webpack.config.js +++ b/src/platform/packages/private/kbn-ui-shared-deps-src/webpack.config.js @@ -23,6 +23,8 @@ const MOMENT_SRC = require.resolve('moment/min/moment-with-locales.js'); const REPO_ROOT = Path.resolve(__dirname, '..', '..', '..', '..', '..'); +const useEuiAmsterdamRelease = process.env.EUI_AMSTERDAM === 'true'; + /** @returns {import('webpack').Configuration} */ module.exports = { externals: { @@ -111,10 +113,21 @@ module.exports = { mainFields: ['browser', 'module', 'main'], conditionNames: ['browser', 'module', 'import', 'require', 'default'], alias: { - '@elastic/eui$': '@elastic/eui/optimize/es', - '@elastic/eui/lib/components/provider/nested$': - '@elastic/eui/optimize/es/components/provider/nested', - '@elastic/eui/lib/services/theme/warning$': '@elastic/eui/optimize/es/services/theme/warning', + // @elastic/eui-amsterdam is a package alias defined in Kibana's package.json + // that points to special EUI releases bundled with Amsterdam set as the default theme + // and meant to be used with Kibana 8.x. Kibana 9.0 and later use the Borealis theme + // and should use the regular @elastic/eui package. + // TODO: Remove when Kibana 8.19 is EOL and Amsterdam backports aren't needed anymore + // https://github.com/elastic/kibana/issues/221593 + '@elastic/eui$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es' + : '@elastic/eui/optimize/es', + '@elastic/eui/lib/components/provider/nested$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es/components/provider/nested' + : '@elastic/eui/optimize/es/components/provider/nested', + '@elastic/eui/lib/services/theme/warning$': useEuiAmsterdamRelease + ? '@elastic/eui-amsterdam/optimize/es/services/theme/warning' + : '@elastic/eui/optimize/es/services/theme/warning', moment: MOMENT_SRC, // NOTE: Used to include react profiling on bundles // https://gist.github.com/bvaughn/25e6233aeb1b4f0cdb8d8366e54a3977#webpack-4 diff --git a/yarn.lock b/yarn.lock index 21a0d251ee1b..3cd58bfe9a8d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2169,6 +2169,47 @@ resolved "https://registry.yarnpkg.com/@elastic/eslint-plugin-eui/-/eslint-plugin-eui-0.2.0.tgz#0f32724e394f3bfa35d18ef8fa2e79da47191779" integrity sha512-EDp5FP8nbzGXwAMWM2xo6SL2LdsUwEVrt6idE8AoW45KybuJZ9Dr8IB8z6+wCr7Jxrg+nwdst039LpaslDur+g== +"@elastic/eui-amsterdam@npm:@elastic/eui@102.2.0-amsterdam.0": + version "102.2.0-amsterdam.0" + resolved "https://registry.yarnpkg.com/@elastic/eui/-/eui-102.2.0-amsterdam.0.tgz#f520caecd95e1054de6b8e21d67f76768ae0b345" + integrity sha512-VqZAac9KHMaPS8yFaibSvD/j2+5/F2/smXrVXIOoTZb7EfK7Fg0UVJOzRdlLon+4W0gHaHvijbe2q8lmym7q6A== + dependencies: + "@elastic/eui-theme-common" "1.0.0" + "@elastic/prismjs-esql" "^1.1.0" + "@hello-pangea/dnd" "^16.6.0" + "@types/lodash" "^4.14.202" + "@types/numeral" "^2.0.5" + "@types/react-window" "^1.8.8" + "@types/refractor" "^3.4.0" + chroma-js "^2.4.2" + classnames "^2.5.1" + lodash "^4.17.21" + mdast-util-to-hast "^10.2.0" + numeral "^2.0.6" + prop-types "^15.8.1" + react-dropzone "^11.7.1" + react-element-to-jsx-string "^15.0.0" + react-focus-on "^3.9.1" + react-is "^17.0.2" + react-remove-scroll-bar "^2.3.4" + react-virtualized-auto-sizer "^1.0.24" + react-window "^1.8.10" + refractor "^3.6.0" + rehype-raw "^5.1.0" + rehype-react "^6.2.1" + rehype-stringify "^8.0.0" + remark-breaks "^2.0.2" + remark-emoji "^2.1.0" + remark-parse-no-trim "^8.0.4" + remark-rehype "^8.1.0" + tabbable "^5.3.3" + text-diff "^1.0.1" + unified "^9.2.2" + unist-util-visit "^2.0.3" + url-parse "^1.5.10" + uuid "^8.3.0" + vfile "^4.2.1" + "@elastic/eui-theme-borealis@1.0.0": version "1.0.0" resolved "https://registry.yarnpkg.com/@elastic/eui-theme-borealis/-/eui-theme-borealis-1.0.0.tgz#f85679d2d72dfc43a620241cbf4161d4e4e81841"