Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
78 changes: 78 additions & 0 deletions e2e/vite-poc-smoke.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { test, expect } from '@playwright/test';

/**
* Smoke test for the Vite PoC dev server (FR-2606).
* Runs against `pnpm --prefix ./react run vite:dev` on port 9081.
*
Comment thread
nowgnuesLee marked this conversation as resolved.
* Not part of CI. Kept on the PoC branch as a reproducer and regression
* guard for the i18n-context-isolation fix (see react/vite-shims/).
*/
Comment thread
nowgnuesLee marked this conversation as resolved.
Comment thread
nowgnuesLee marked this conversation as resolved.
test('Vite PoC: app mounts at :9081 with host i18n translations rendered', async ({
page,
}) => {
test.skip(
!!process.env.CI || !process.env.VITE_POC_SMOKE,
'Local opt-in smoke test for a separately started Vite dev server (set VITE_POC_SMOKE=1 to run)',
);

// Pre-existing warnings unrelated to the Vite migration. Keep this list
// short — new entries that look Vite-specific should fail the test.
const KNOWN_PREEXISTING_WARNINGS = [
// react-dom warns about some antd CSS-in-JS vendor prefixes under React 19.
'Unsupported style property %s. Did you mean %s? -webkit-app-region',
// antd v6 deprecation notice that is tracked independently.
'antd: Dropdown',
];

const pageErrors: string[] = [];
const networkFailures: Array<{ url: string; status: number }> = [];
const filteredErrors: string[] = [];

page.on('console', (msg) => {
if (msg.type() !== 'error') return;
const text = msg.text();
if (KNOWN_PREEXISTING_WARNINGS.some((k) => text.includes(k))) return;
filteredErrors.push(text);
});
page.on('pageerror', (err) => {
pageErrors.push(`${err.message}\n${err.stack ?? ''}`);
});
page.on('response', (resp) => {
if (resp.status() >= 400) {
networkFailures.push({ url: resp.url(), status: resp.status() });
}
});

await page.goto('http://127.0.0.1:9081/', { waitUntil: 'domcontentloaded' });
Comment thread
nowgnuesLee marked this conversation as resolved.

// Wait for React to mount the login form.
await page
.getByRole('button', { name: /session|api|login/i })
.first()
.waitFor({ timeout: 15_000 });

const bodyText = await page.evaluate(() => document.body.innerText ?? '');

// The regression the shim fix targets: host app keys (`login.X`) were
// rendering as raw strings instead of resolved translations because
// BUI's <I18nextProvider> was leaking into the host tree under Vite's
// deduped module graph. Asserting that the rendered text does NOT
// contain raw keys is the core regression guard.
expect(
bodyText,
'host i18n translations must be resolved, not rendered as keys',
).not.toMatch(/\blogin\.[A-Za-z]+/);

// A few positive-example translations that should appear on the login
// page in English. (If the page adds/removes these someday, adjust the
// list — the negative match above is the primary check.)
expect(bodyText).toMatch(/Forgot password\?/);
expect(bodyText).toMatch(/Sign up/);

expect(pageErrors, 'no uncaught page errors').toEqual([]);
expect(networkFailures, 'no failed network requests').toEqual([]);
expect(
filteredErrors,
'no console errors beyond the known-pre-existing set',
).toEqual([]);
});
20 changes: 18 additions & 2 deletions react/VITE_POC_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,24 @@ These appeared during the PoC and should be tracked separately:

1. **`PluginLoader.tsx` dynamic import warning** — `import(pluginUrl /* webpackIgnore: true */)` is not understood by Vite. Needs `/* @vite-ignore */` in the Vite migration (or a different plugin-loading strategy).
2. **Static resources are served via a small custom middleware**. Craco's dev server had richer behaviour (fs.watch on `config.toml`, `resources/i18n/**`, `resources/theme.json` → full page reload). A follow-up sub-issue should port that behaviour to a Vite plugin.
3. **No visual sanity check** — this PoC verified every transform and HMR mechanism by curl + log inspection, but we did not open the app in a browser and confirm it mounts end-to-end against a running Backend.AI cluster. A short Playwright smoke test would close that gap.
4. **`backend.ai-client-esm` watcher behaviour** — the alias resolves, but the root `tsc --watch` → Vite re-serve loop was not stress-tested.
3. **`backend.ai-client-esm` watcher behaviour** — the alias resolves, but the root `tsc --watch` → Vite re-serve loop was not stress-tested.
4. **Pre-existing console warnings surfaced by the smoke test** — `antd: Dropdown overlayStyle` (antd v6 migration) and `-webkit-app-region` (antd-style + React 19 warning). These existed before the Vite migration but are now visible; the smoke test allowlists them.

## i18n isolation — extra Phase 1 work landed here

Initial browser smoke showed host-app translation keys (`login.SessionMode`, etc.) rendering as raw strings. Root cause diagnosed and fixed:

- BUI designs for two separate `i18next` instances (host + BUI) — each `<I18nextProvider>` is supposed to scope to a different React Context object. Under CRA this worked because pnpm splits `react-i18next` by peer-dep version (typescript@5.7.3 for react/, @5.9.3 for BUI), giving two physical copies of the module, each with its own `React.createContext(...)`.
- Vite's dep optimizer dedupes by bare name, collapsing both copies into one bundled chunk → one Context object. BUI's `<I18nextProvider>` then leaks into host components and resolves host keys against BUI's resource set (BUI keys only) → raw keys render.

Fix:

1. `optimizeDeps.exclude: ['i18next', 'react-i18next']` — skip these from Vite's dep pre-bundling so each pnpm store path remains a distinct module.
2. Add ESM shims (`react/vite-shims/void-elements.mjs`, `use-sync-external-store-shim.mjs`) aliased to replace the CJS transitive deps of `react-i18next`. Without these shims, excluding `react-i18next` leaves those leaves without CJS→ESM transform and the browser's native ESM parser rejects them.
3. Verified via Playwright smoke (`e2e/vite-poc-smoke.spec.ts`):
- `packages/backend.ai-ui/src/components/BAIText.tsx` resolves `react-i18next` from the `typescript@5.9.3` store.
- `react/src/components/LoginView.tsx` resolves `react-i18next` from the `typescript@5.7.3` store.
- Host page renders English translations (not raw keys).

## Not validated in this PoC

Expand Down
15 changes: 15 additions & 0 deletions react/vite-shims/use-sync-external-store-shim.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// ESM shim for `use-sync-external-store/shim` (CJS-only export path).
//
// Why this exists: the Vite PoC excludes `react-i18next` from dep
// optimization (to preserve two physical copies for BUI i18n context
// isolation). `react-i18next`'s ESM bundle imports
// `useSyncExternalStore` from `use-sync-external-store/shim`, which is a
// CJS file with `process.env.NODE_ENV` branching and `require(...)`
// dispatch. When its parent is excluded, Vite's optimizer doesn't
// transform it, and the browser fails to import it as ESM.
//
// This shim re-exports React's native `useSyncExternalStore` (stable
// since 18.0). The WebUI targets React 19.x, so the shim package's
// React-17-compat polyfill is not needed — the native hook is identical
// in behaviour for our use cases.
export { useSyncExternalStore } from 'react';
33 changes: 33 additions & 0 deletions react/vite-shims/void-elements.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// ESM shim for `void-elements` (CJS-only package).
//
// Why this exists: the Vite PoC excludes `react-i18next` from dep
// optimization so two physical copies can exist (one per pnpm peer-version
// store path) — that's what keeps BUI's `<I18nextProvider>` context
// isolated from the host tree. The side effect is that
// `html-parse-stringify`'s ESM file does `import e from "void-elements"`,
// and `void-elements@3.1.0` is pure CJS (`module.exports = {...}`, no ESM
// variant). When its parent is excluded, Vite's optimizer never converts
// this leaf to ESM — browsers then error with
// "does not provide an export named 'default'".
//
// This shim replaces the import entirely. `void-elements` is a plain
// lookup object of HTML void-element tag names; reproducing it as ESM
// is trivial and avoids running any CJS interop at all.
//
// Kept in sync with node_modules/.pnpm/void-elements@3.1.0/.../index.js.
export default {
area: true,
base: true,
br: true,
col: true,
embed: true,
hr: true,
img: true,
input: true,
link: true,
meta: true,
param: true,
source: true,
track: true,
wbr: true,
};
63 changes: 62 additions & 1 deletion react/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ export default defineConfig(({ mode }) => {
cacheDir: resolve(__dirname, 'node_modules/.vite'),

resolve: {
// Force a single instance of shared singletons across the monorepo.
//
// BUI intentionally maintains its OWN i18n instance (see the note in
// packages/backend.ai-ui/vite.config.ts). Do NOT add `i18next` or
// `react-i18next` here — the design relies on each side having its
// own default singleton. React, Relay, Jotai must still be deduped
// so hooks/state sharing works across the monorepo.
dedupe: [
'react',
'react-dom',
'react-relay',
'relay-runtime',
'jotai',
],
// Array form lets us mix regex aliases (for `src/` baseUrl imports) with
// the workspace-package aliases. `find` is matched against the import
// source; `replacement` replaces the match.
Expand All @@ -156,6 +170,33 @@ export default defineConfig(({ mode }) => {
find: /^backend\.ai-client-esm$/,
replacement: resolve(projectRoot, 'dist/lib/backend.ai-client-esm.js'),
},

// ESM shims for CJS-only transitive deps of `react-i18next`.
//
// Why: `react-i18next` and `i18next` are intentionally excluded
// from Vite's dep optimizer (see `optimizeDeps.exclude` below) so
// two physical copies can coexist — that's what preserves BUI's
// i18n Context isolation from the host app. The side effect is
// that their CJS transitive deps (`void-elements`,
// `use-sync-external-store/shim`) never go through the
// optimizer's CJS→ESM transform, which breaks the browser's
// native ESM `import default from 'cjs-pkg'` expectation.
//
// These aliases redirect those imports to hand-written ESM
// shims under `react/vite-shims/` — `void-elements` is a plain
// lookup object, and `use-sync-external-store/shim` re-exports
// React 19's native `useSyncExternalStore`.
{
find: /^void-elements$/,
replacement: resolve(__dirname, 'vite-shims/void-elements.mjs'),
},
{
find: /^use-sync-external-store\/shim$/,
replacement: resolve(
__dirname,
'vite-shims/use-sync-external-store-shim.mjs',
),
},
],
},

Expand All @@ -168,7 +209,27 @@ export default defineConfig(({ mode }) => {
'src/index.tsx',
'src/**/*.{ts,tsx}',
],
exclude: ['backend.ai-ui'],
exclude: [
'backend.ai-ui',
// BUI is designed to have its OWN i18n singleton separate from the
// host app (see packages/backend.ai-ui/src/locale/index.ts +
// BAIConfigProvider wraps children with <I18nextProvider i18n={buiI18n}>).
//
// Under CRA this worked because pnpm's per-package i18next/react-i18next
// copies (split by typescript peer version) meant both `react-i18next`
// copies had DIFFERENT React Context objects. BUI's Provider published
// to its own Context; host's useTranslation read host's Context.
//
// Under Vite's dep optimizer the two copies collapse into one bundled
// module = one Context object. BAIConfigProvider's Provider then
// leaks into host components and they resolve host keys against BUI's
// resource set (which only has BUI keys) → raw keys render.
//
// Excluding from optimization preserves natural pnpm-per-package
// resolution and keeps the two Context objects distinct.
'i18next',
'react-i18next',
],
},

server: {
Expand Down
Loading