From b126621b3b59917a0755f7406e601e4d5bb8d7a4 Mon Sep 17 00:00:00 2001 From: Rian Polonini <87614124+RianPolonini@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:38:01 -0300 Subject: [PATCH] perf: load i18n messages on demand in vitest setup (#15291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `vitest.setup.js` runs for every spec file and builds the i18n instance from the full message catalogue, so each of the 389 spec files resolves and transforms the 2537 JSON files under `app/javascript/dashboard/i18n/locale/`. With `pool: 'threads'`, that cost is paid per worker. The result is that `setup` takes about **60x longer than the tests themselves**. Only one spec in the suite asserts on translated copy. This PR leaves the global i18n instance without messages and adds `withFullI18n`, an opt-in helper for the specs that need the real catalogue. **On this repository's own CI, `setup` drops from ~629s to ~71s (-89%) and total Vitest duration from ~388s to ~196s (-49%).** This is a performance and testability change, not a cosmetic one. It touches only how the test harness loads messages — **no translatable string and no locale file is modified**, so nothing changes for Crowdin contributors. Worth noting: `vitest.config.ts` already excludes `**/i18n/**/*` from coverage, so loading the catalogue in every spec was not serving any metric. Fixes #15290 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### On this repository's CI The numbers below come from the `test` job of `frontend-fe.yml` on GitHub Actions — this repo's own runners, not a local machine. Baseline is **60 successful `develop` runs** (23–31 Jul 2026), parsed from the Vitest summary line in each job log. | Vitest metric | `develop` (n=60) | This PR | Delta | |---|---|---|---| | **setup** | median **629s** (min 329 / p25 607 / p75 641 / max 680) | **71s** | **-559s (-89%)** | | **duration** | median **388s** (min 205 / p25 371 / p75 393 / max 418) | **196s** | **-192s (-49%)** | None of the 60 `develop` runs beat this PR on either metric. Same commit, both on this repo's CI: ``` develop @ bc7ae88 Test Files 389 passed (389) Duration 323.35s (transform 13.75s, setup 522.48s, collect 42.53s, tests 9.02s, environment 187.05s, prepare 32.77s) this PR Test Files 389 passed (389) Duration 196.20s (transform 18.18s, setup 70.67s, collect 65.97s, tests 12.22s, environment 251.28s, prepare 43.90s) ``` `setup` is the metric that isolates this change: it is a sum of work, so it is not distorted by how fast a given runner happens to be. `duration` improves less because it also covers transform, collect and environment, which this change does not touch. One caveat if you compare total job times instead: on this PR's run, `ruby/setup-ruby` took 130s versus 11s on the baseline (an unrelated cache miss), which hides most of the gain at job level. ### Locally `pnpm exec vitest run`, twice per scenario, on `develop` at `bc7ae88` (Node 24.18.1, macOS arm64): | | setup | duration | |---|---|---| | before | 373s / 412s | 60.8s / 67.8s | | after | 40.5s / 42.8s | 28.1s / 28.5s | Identical pass/fail counts before and after. Locally there is one failure in both scenarios, pre-existing on `develop` and unrelated to this change: `useReportMetrics.spec.js` expects `'5,000'` and receives `'5.000'`, a thousands-separator difference that depends on the machine locale. It does not occur on CI, where the suite is fully green (389/389). `MacroProperties.spec.js` was the only spec that depended on the global catalogue — it asserts the real copy from `macros.json`. It now calls `withFullI18n()` and keeps asserting the same strings, so coverage of that copy is preserved. ## Notes for reviewers - The `test-i18n` alias was added to `vitest.config.ts` rather than to `vite.shared.ts`, to keep it out of the production build. - `missingWarn: false` and `fallbackWarn: false` were added to the global instance so specs that render translated components without opting in do not flood the output with missing-key warnings. - Any future spec that needs the real copy just calls `withFullI18n()` at the top of the file. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works (no new test: this is a performance change, verified by the CI measurements above and by the unchanged pass/fail counts) - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules Co-authored-by: Shivam Mishra --- .../macros/specs/MacroProperties.spec.js | 3 +++ vitest.config.ts | 5 ++++- vitest.i18n.js | 21 +++++++++++++++++++ vitest.setup.js | 7 +++++-- 4 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 vitest.i18n.js diff --git a/app/javascript/dashboard/routes/dashboard/settings/macros/specs/MacroProperties.spec.js b/app/javascript/dashboard/routes/dashboard/settings/macros/specs/MacroProperties.spec.js index 31218cbba..edd1b53d6 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/macros/specs/MacroProperties.spec.js +++ b/app/javascript/dashboard/routes/dashboard/settings/macros/specs/MacroProperties.spec.js @@ -1,6 +1,9 @@ import { shallowMount } from '@vue/test-utils'; +import { withFullI18n } from 'test-i18n'; import MacroProperties from '../MacroProperties.vue'; +withFullI18n(); + const mountComponent = props => shallowMount(MacroProperties, { props: { diff --git a/vitest.config.ts b/vitest.config.ts index 5533f3998..6656bb159 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,5 @@ /// +import path from 'path'; import { defineConfig } from 'vitest/config'; import vue from '@vitejs/plugin-vue'; import { aliases, vueOptions } from './vite.shared'; @@ -6,7 +7,9 @@ import yaml from '@rollup/plugin-yaml'; export default defineConfig({ plugins: [vue(vueOptions), yaml()], - resolve: { alias: aliases }, + resolve: { + alias: { ...aliases, 'test-i18n': path.resolve('./vitest.i18n.js') }, + }, test: { environment: 'jsdom', include: ['app/**/*.{test,spec}.?(c|m)[jt]s?(x)'], diff --git a/vitest.i18n.js b/vitest.i18n.js new file mode 100644 index 000000000..f0ef7d4ca --- /dev/null +++ b/vitest.i18n.js @@ -0,0 +1,21 @@ +import { config } from '@vue/test-utils'; +import { createI18n } from 'vue-i18n'; +import i18nMessages from 'dashboard/i18n'; +import FloatingVue from 'floating-vue'; + +/** + * Replaces the empty i18n instance from the global setup with the real + * messages. Call it at the top of the spec, outside of hooks. + * See vitest.setup.js. + */ +export function withFullI18n(locale = 'en') { + const i18n = createI18n({ + legacy: false, + locale, + messages: i18nMessages, + }); + + config.global.plugins = [i18n, FloatingVue]; + + return i18n; +} diff --git a/vitest.setup.js b/vitest.setup.js index 1bed8ee3e..12516be93 100644 --- a/vitest.setup.js +++ b/vitest.setup.js @@ -1,12 +1,15 @@ import { config } from '@vue/test-utils'; import { createI18n } from 'vue-i18n'; -import i18nMessages from 'dashboard/i18n'; import FloatingVue from 'floating-vue'; +// No messages: loading them here would pull 2537 locale files into every spec. +// Specs asserting on translated copy should use withFullI18n from vitest.i18n. const i18n = createI18n({ legacy: false, locale: 'en', - messages: i18nMessages, + messages: {}, + missingWarn: false, + fallbackWarn: false, }); config.global.plugins = [i18n, FloatingVue];