From 10cc4bdd3aae31f80a2a3728a03a01aa1331e4fb Mon Sep 17 00:00:00 2001 From: Teo Koon Peng Date: Tue, 6 Aug 2024 11:09:57 +0800 Subject: [PATCH] dashboard: fix and improve build (#986) * fix deps not built before building dashboard Signed-off-by: Teo Koon Peng * cleanup old config; make adminTab and customTab optional Signed-off-by: Teo Koon Peng * allow authenticator to be tree shaked; rudimetry way to have runtime args Signed-off-by: Teo Koon Peng * allow custom tabs and admin tags to be tree shaken Signed-off-by: Teo Koon Peng * actually tree shake out admin tab Signed-off-by: Teo Koon Peng * put all build time config in separate object Signed-off-by: Teo Koon Peng * fallback to test config Signed-off-by: Teo Koon Peng * add base url to build config Signed-off-by: Teo Koon Peng * add __init__.py so older setuptools sees ros_pydantic Signed-off-by: Teo Koon Peng * use header logo in login page Signed-off-by: Teo Koon Peng --------- Signed-off-by: Teo Koon Peng --- package.json | 4 +- .../models/ros_pydantic/__init__.py | 0 packages/dashboard/app-config.d.ts | 56 ++++++++++++++ packages/dashboard/app-config.json | 14 ++-- packages/dashboard/index.html | 2 +- packages/dashboard/package.json | 2 +- packages/dashboard/src/app-config.ts | 73 ++++++++++--------- packages/dashboard/src/components/app.tsx | 56 +++++++------- packages/dashboard/src/components/appbar.tsx | 4 +- packages/dashboard/src/vite-env.d.ts | 5 ++ packages/dashboard/vite.config.ts | 48 +++++++++++- .../ros_translator/pydantic/__init__.py | 7 +- pnpm-lock.yaml | 12 +-- 13 files changed, 204 insertions(+), 79 deletions(-) create mode 100644 packages/api-server/api_server/models/ros_pydantic/__init__.py create mode 100644 packages/dashboard/app-config.d.ts diff --git a/package.json b/package.json index 1e2e2dd0d..647e597e4 100644 --- a/package.json +++ b/package.json @@ -27,8 +27,8 @@ "prettier --write" ], "**/*.py": [ - ".venv/bin/isort", - ".venv/bin/black" + ".venv/bin/pipenv run isort", + ".venv/bin/pipenv run black" ] }, "overrides": { diff --git a/packages/api-server/api_server/models/ros_pydantic/__init__.py b/packages/api-server/api_server/models/ros_pydantic/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/packages/dashboard/app-config.d.ts b/packages/dashboard/app-config.d.ts new file mode 100644 index 000000000..34ddfbb1b --- /dev/null +++ b/packages/dashboard/app-config.d.ts @@ -0,0 +1,56 @@ +export interface RobotResource { + icon?: string; + scale?: number; +} + +export interface FleetResource { + default: RobotResource; +} + +export interface LogoResource { + header: string; +} + +export interface Resources { + fleets: Record; + logos: LogoResource; +} + +export interface TaskResource { + taskDefinitionId: string; + displayName?: string; +} + +export interface StubAuthConfig { + provider: 'stub'; + config: null; +} + +export interface KeycloakAuthConfig { + provider: 'keycloak'; + config: { + url: string; + realm: string; + clientId: string; + }; +} + +export interface AppConfigInput { + rmfServerUrl: string; + trajectoryServerUrl: string; + auth: KeycloakAuthConfig | StubAuthConfig; + helpLink: string; + reportIssue: string; + pickupZones: string[]; + defaultZoom: number; + defaultRobotZoom: number; + attributionPrefix: string; + defaultMapLevel: string; + allowedTasks: TaskResource[]; + resources: Record & Record<'default', Resources>; + customTabs?: boolean; + adminTab?: boolean; + // FIXME(koonpeng): this is used for very specific tasks, should be removed when mission + // system is implemented. + cartIds: string[]; +} diff --git a/packages/dashboard/app-config.json b/packages/dashboard/app-config.json index 2c41dacc1..7397b8dfe 100644 --- a/packages/dashboard/app-config.json +++ b/packages/dashboard/app-config.json @@ -1,9 +1,7 @@ { "rmfServerUrl": "http://localhost:8000", "trajectoryServerUrl": "ws://localhost:8006", - "auth": { - "provider": "stub" - }, + "authConfig": {}, "helpLink": "https://osrf.github.io/ros2multirobotbook/rmf-core.html", "reportIssue": "https://github.com/open-rmf/rmf-web/issues", "pickupZones": [], @@ -33,7 +31,11 @@ } } }, - "customTabs": false, - "adminTab": false, - "cartIds": [] + "cartIds": [], + "buildConfig": { + "baseUrl": "/", + "authProvider": "stub", + "customTabs": false, + "adminTab": false + } } diff --git a/packages/dashboard/index.html b/packages/dashboard/index.html index a5b8abaea..48f3e651a 100644 --- a/packages/dashboard/index.html +++ b/packages/dashboard/index.html @@ -1,4 +1,4 @@ - + diff --git a/packages/dashboard/package.json b/packages/dashboard/package.json index aa835aaec..10542c49a 100644 --- a/packages/dashboard/package.json +++ b/packages/dashboard/package.json @@ -4,7 +4,7 @@ "type": "module", "private": true, "scripts": { - "build": "vite build", + "build": "pnpm run --filter {.}^... build && vite build", "build-storybook": "storybook build", "lint": "tsc --build && eslint --max-warnings 0 src", "start": "concurrently npm:start:rmf-server npm:start:react", diff --git a/packages/dashboard/src/app-config.ts b/packages/dashboard/src/app-config.ts index da7a7032a..43fecdaed 100644 --- a/packages/dashboard/src/app-config.ts +++ b/packages/dashboard/src/app-config.ts @@ -1,7 +1,7 @@ import React from 'react'; import { getDefaultTaskDefinition, TaskDefinition } from 'react-components'; -import appConfigJson from '../app-config.json'; +import testConfig from '../app-config.json'; import { Authenticator, KeycloakAuthenticator, StubAuthenticator } from './auth'; import { BasePath } from './util/url'; @@ -28,27 +28,18 @@ export interface TaskResource { displayName?: string; } -export interface StubAuthConfig { - provider: 'stub'; -} +export interface StubAuthConfig {} export interface KeycloakAuthConfig { - provider: 'keycloak'; - config: { - url: string; - realm: string; - clientId: string; - }; -} - -export interface AuthConfig { - provider: string; + url: string; + realm: string; + clientId: string; } -export interface AppConfig { +export interface RuntimeConfig { rmfServerUrl: string; trajectoryServerUrl: string; - auth: KeycloakAuthConfig | StubAuthConfig; + authConfig: KeycloakAuthConfig | StubAuthConfig; helpLink: string; reportIssue: string; pickupZones: string[]; @@ -58,32 +49,48 @@ export interface AppConfig { defaultMapLevel: string; allowedTasks: TaskResource[]; resources: Record & Record<'default', Resources>; - customTabs: boolean; - adminTab: boolean; // FIXME(koonpeng): this is used for very specific tasks, should be removed when mission // system is implemented. cartIds: string[]; } -// we specifically don't export app config to force consumers to use the context. -const appConfig: AppConfig = appConfigJson as AppConfig; +// these will be injected as defines and potentially be tree shaken out +export interface BuildConfig { + baseUrl: string; + authProvider: 'keycloak' | 'stub'; + customTabs?: boolean; + adminTab?: boolean; +} + +export interface AppConfig extends RuntimeConfig { + buildConfig: BuildConfig; +} + +declare const APP_CONFIG: AppConfig; + +const appConfig: AppConfig = (() => { + if (import.meta.env.PROD) { + return APP_CONFIG; + } else { + // globals cannot be injected in tests so we need a fallback, this should be + // removed by terser in prod builds. + return testConfig as AppConfig; + } +})(); export const AppConfigContext = React.createContext(appConfig); const authenticator: Authenticator = (() => { - switch (appConfig.auth.provider) { - case 'keycloak': - if (!import.meta.env.VITE_KEYCLOAK_CONFIG) { - throw new Error('missing VITE_KEYCLOAK_CONFIG'); - } - return new KeycloakAuthenticator( - appConfig.auth.config, - `${window.location.origin}${BasePath}/silent-check-sso.html`, - ); - case 'stub': - return new StubAuthenticator(); - default: - throw new Error('unknown auth provider'); + // must use if statement instead of switch for vite tree shaking to work + if (APP_CONFIG_AUTH_PROVIDER === 'keycloak') { + return new KeycloakAuthenticator( + APP_CONFIG.authConfig as KeycloakAuthConfig, + `${window.location.origin}${BasePath}/silent-check-sso.html`, + ); + } else if (APP_CONFIG_AUTH_PROVIDER === 'stub') { + return new StubAuthenticator(); + } else { + throw new Error('unknown auth provider'); } })(); diff --git a/packages/dashboard/src/components/app.tsx b/packages/dashboard/src/components/app.tsx index 3db035d81..85dbf8093 100644 --- a/packages/dashboard/src/components/app.tsx +++ b/packages/dashboard/src/components/app.tsx @@ -96,32 +96,38 @@ export default function App(): JSX.Element | null { } /> - - - - } - /> + {APP_CONFIG_ENABLE_CUSTOM_TABS && ( + + + + } + /> + )} - - - - } - /> + {APP_CONFIG_ENABLE_CUSTOM_TABS && ( + + + + } + /> + )} - - - - } - /> + {APP_CONFIG_ENABLE_ADMIN_TAB && ( + + + + } + /> + )} @@ -132,7 +138,7 @@ export default function App(): JSX.Element | null { element={ authenticator.login(`${window.location.origin}${DashboardRoute}`) } diff --git a/packages/dashboard/src/components/appbar.tsx b/packages/dashboard/src/components/appbar.tsx index dd8f69d71..4487642cb 100644 --- a/packages/dashboard/src/components/appbar.tsx +++ b/packages/dashboard/src/components/appbar.tsx @@ -344,7 +344,7 @@ export const AppBar = React.memo(({ extraToolbarItems }: AppBarProps): React.Rea aria-label="Tasks" onTabClick={() => navigate(TasksRoute)} /> - {appConfig.customTabs && ( + {APP_CONFIG_ENABLE_CUSTOM_TABS && ( <> )} - {appConfig.adminTab && profile?.user.is_admin && ( + {APP_CONFIG_ENABLE_ADMIN_TAB && profile?.user.is_admin && ( + +declare const APP_CONFIG_BASE_URL: string; +declare const APP_CONFIG_AUTH_PROVIDER: string; +declare const APP_CONFIG_ENABLE_CUSTOM_TABS: boolean; +declare const APP_CONFIG_ENABLE_ADMIN_TAB: boolean; diff --git a/packages/dashboard/vite.config.ts b/packages/dashboard/vite.config.ts index 67ec02b6f..6cc10bff3 100644 --- a/packages/dashboard/vite.config.ts +++ b/packages/dashboard/vite.config.ts @@ -1,11 +1,55 @@ /// import react from '@vitejs/plugin-react-swc'; -import { defineConfig } from 'vite'; +import { defineConfig, Plugin } from 'vite'; + +import appConfig from './app-config.json'; + +/** + * The goal of this plugin is to inject global variables to `index.html`, allowing + * a crude way to configure some variables after the bundle is built via `sed`. + * + * An example use case is building a dockerfile, because the domain in a dev or staging + * environment is typically different from prod, we would normally end up needing to build + * different images (which is really detrimental for staging). In such scenario, you can + * set the `rmfServerUrl` to a placeholder like `__RMF_SERVER_URL_PLACEHOLDER__` in + * `app-config.json`, then do a search and replace at the container entrypoint. + * + * The reason for doing this outside the app code is to avoid these variables from + * being modified by the bundler, and to reduce the chance that the search and replace modify + * unintended code. + */ +const injectGlobals: Plugin = { + name: 'injectRuntimeArgs', + transformIndexHtml: { + order: 'pre', // must be injected before the app + handler: () => { + return [ + { + tag: 'script', + injectTo: 'head', + children: `const APP_CONFIG=${JSON.stringify(appConfig)}`, + }, + ]; + }, + }, +}; + +function booleanToString(b: boolean | null | undefined) { + return b ? 'true' : 'false'; +} + +const buildConfig = appConfig.buildConfig; // https://vitejs.dev/config/ export default defineConfig({ - plugins: [react()], + base: buildConfig.baseUrl, + define: { + APP_CONFIG_AUTH_PROVIDER: `'${buildConfig.authProvider}'`, + APP_CONFIG_ENABLE_CUSTOM_TABS: `${booleanToString(buildConfig.customTabs)}`, + APP_CONFIG_ENABLE_ADMIN_TAB: `${booleanToString(buildConfig.adminTab)}`, + }, + plugins: [injectGlobals, react()], test: { environment: 'jsdom', globals: true, diff --git a/packages/ros-translator/ros_translator/pydantic/__init__.py b/packages/ros-translator/ros_translator/pydantic/__init__.py index f17060eb0..35fe055df 100644 --- a/packages/ros-translator/ros_translator/pydantic/__init__.py +++ b/packages/ros-translator/ros_translator/pydantic/__init__.py @@ -120,7 +120,9 @@ def generate_messages(roslib: RosLibrary, pkg: str, outdir: str): def generate_init(namespace: Namespace, outdir: str): - with open(joinp(outdir, namespace.full_name, "__init__.py"), mode="w") as f: + with open( + joinp(outdir, namespace.full_name, "__init__.py"), mode="w", encoding="utf8" + ) as f: for m in namespace.messages.values(): name = m.structure.namespaced_type.name f.write(f"from .{name} import {name}\n") @@ -140,6 +142,9 @@ def generate_modules(pkgs: Sequence[str], outdir: str): generate_messages(roslib, pkg_index.pkg_name, outdir) generate_init(pkg_index.root_ns, outdir) + with open(joinp(outdir, "__init__.py"), mode="w", encoding="utf8"): + pass + def generate(pkgs: Sequence[str], outdir: str): print("Generating pydantic interfaces") diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 497774d0d..f6179cfb3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -74,7 +74,7 @@ importers: version: 5.4.5 vitest: specifier: ^2.0.4 - version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3) + version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3) packages/api-server: devDependencies: @@ -249,7 +249,7 @@ importers: version: 5.3.5(@types/node@20.14.12)(terser@5.31.3) vitest: specifier: ^2.0.4 - version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3) + version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3) packages/dashboard-e2e: devDependencies: @@ -499,7 +499,7 @@ importers: version: 7.16.0(eslint@8.57.0)(typescript@5.4.5) vitest: specifier: ^2.0.4 - version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3) + version: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3) packages/rmf-models: devDependencies: @@ -10327,7 +10327,7 @@ snapshots: '@jest/globals': 29.7.0 '@types/jest': 29.5.12 jest: 29.7.0(@types/node@20.14.12) - vitest: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3) + vitest: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3) '@testing-library/react@14.3.1(react-dom@18.3.1(react@18.3.1))(react@18.3.1)': dependencies: @@ -10824,7 +10824,7 @@ snapshots: std-env: 3.7.0 test-exclude: 7.0.1 tinyrainbow: 1.2.0 - vitest: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3) + vitest: 2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3) transitivePeerDependencies: - supports-color @@ -16398,7 +16398,7 @@ snapshots: fsevents: 2.3.3 terser: 5.31.3 - vitest@2.0.4(@types/node@20.14.12)(jsdom@24.1.1)(terser@5.31.3): + vitest@2.0.4(@types/node@20.14.12)(jsdom@24.1.1(canvas@2.11.2))(terser@5.31.3): dependencies: '@ampproject/remapping': 2.3.0 '@vitest/expect': 2.0.4