Migrate client build to Vite/Vitest, add TypeScript, and enhance testing#233
Conversation
|
One important side comment which I have forgotten to include. Currently the typescript compiler's type and rule checking is set to rather relaxed (e.g. we allow Now of course this is not recommended in general, I personally like to use the strictest type checking as it can catch tons of bugs up front but that some may find cumber some especially at the beginning and when doing prototyping work and one does not want the overhead of strictly typing everything. What I am trying to say is that we can leave it like this or we can harden the checkings and I can also integrate the errors into vite dev server/build meaning that even the build will fail if not all TS errors are resolved. Currently there is one type error with the action command type as the interval setting uses data which is not present to prove the point (and also to remind me that I need to improve the type interface of the the payload object). |
First off: great work my friend. Just great work. Can we follow the same approach as we use with ESLint: set it to nonblocking 'warning' first to make us aware of issues, and once we cleared the issues, set it to generate errors? We have some time before releasing 0.9.8, as I expect some engine changes whereas my simulation environment still is working on testing 0.9.7C. So there wouldn't be much stress to modify code all in one go. Practical question: I have one outstanding issue with @DXCanas PR, but your PR builds on top of them? And perhaps rebase against 0.9.8 branch, instead directly into 0.9.7/main? Ibrather gather all changes in one bigger build and test thoroughly before release.
Can we migrate slowly to ts everywhere, or will we need to go big bang somewhere? |
- Replace rollup -c with vite build in package.json scripts - Add vite.config.js with root: app/client, output to build/ - Add esbuild plugin for JS decorator support in dev mode - Move manifest.json and icon.png to public/ dir (static assets) - Update index.html asset references to absolute paths - Add 'dev' script for Vite dev server with HMR - Install vite and esbuild as devDependencies
- Add tsconfig.json with strict settings and experimental decorators - Replace @babel/eslint-parser with @typescript-eslint/parser - Add @typescript-eslint/eslint-plugin for TS-aware linting - Add eslint-plugin-lit and eslint-plugin-wc for Lit/WC rules - Consolidate app/client/.eslintrc.json into root eslint.config.js - Add typecheck script to package.json - Fix pre-existing duplicate ignoreEOLComments key in eslint config
Rename all 15 client .js files to .ts and add type annotations
throughout. Remove 'use strict' directives (implied by ES modules),
remove accessor keyword from Lit decorator properties (using
experimentalDecorators), and remove .js extensions from intra-client
imports (bundler moduleResolution).
Update ESLint config to use the documented flat config pattern for
eslint-plugin-lit and eslint-plugin-wc (named {configs} import with
spread). Add TypeScript-specific overrides to disable no-undef (handled
by tsc) and replace no-unused-vars with @typescript-eslint/no-unused-vars
for .ts files. Remove duplicate @eslint/js import and defineConfig
wrapper.
Remove the temporary jsDecoratorPlugin from vite.config.js and the
allowJs/config file includes from tsconfig.json that were only needed
during the mixed JS/TS transition.
The new packages supports ESM module imports and is a fork of the original nosleep.js package, which has not been updated for a while.
Install vitest and convert helper.test.js to helper.test.ts, replacing uvu imports (test, assert.equal) with vitest equivalents (test, expect, toEqual). Remove the uvu-specific test.run() call. Add test:client script to package.json for running client tests with vitest separately from the backend uvu tests.
Delete rollup.config.js, babel.config.json, and jsconfig.json (replaced by vite.config.js and tsconfig.json). Remove devDependencies no longer needed: rollup, @rollup/plugin-babel, @rollup/plugin-commonjs, @rollup/plugin-node-resolve, @rollup/plugin-terser, @web/rollup-plugin-html, rollup-plugin-summary, @babel/plugin-proposal-decorators, @babel/preset-env, and esbuild. Scope the uvu test command to ignore the client directory (now tested via vitest) using the -i flag.
Add store/types.ts with comprehensive TypeScript interfaces derived from backend WebSocket message shapes: AppState, RowingMetrics, MetricsContext, IntervalData, AppConfig, GuiConfig, and DashboardMetricDefinition. Update all consuming files to import and use proper types: - appState.ts: type APP_STATE as AppState - dashboardMetrics.ts: use DashboardMetricDefinition type - PerformanceDashboard.ts: appState typed as AppState - DashboardToolbar.ts: config typed as AppConfig - SettingsDialog.ts: config typed as GuiConfig - app.ts: AppInterface uses AppState, resetFields preserves heartrate fields via destructuring instead of generic filterObjectByKeys - index.ts: _appState typed as AppState, updateState accepts Partial<AppState> Optional fields in RowingMetrics and IntervalData reflect that the initial APP_STATE has only a subset of fields before the first WebSocket message arrives.
…, and dialog 95 tests across 5 test files (up from 1 test in 1 file): - helper.test.ts: complete tests for filterObjectByKeys, secondsToTimeString, formatDistance, and formatNumber (27 tests) - workout-utils.test.ts: distance/time/calories format functions and buildWorkoutPlan interval construction (13 tests) - dashboardMetrics.test.ts: structure validation, template rendering for all 16 metrics, interval type logic, stroke ratio edge cases (31 tests) - DashboardToolbar.test.ts: blePeripheralMode mapping for all BLE peripheral modes (7 tests) - WorkoutDialog.test.ts: rendering, increment accumulation, reset, and close/confirm behavior (17 tests) Extract workout config and plan-building logic into workout-utils.ts for testability without DOM dependencies. Refactor WorkoutDialog to import from workout-utils instead of duplicating config. Add happy-dom as dev dependency for Lit component tests.
Add 45 new tests across 9 test files covering: - AppElement: sendEvent dispatches CustomEvent with bubbles/composed - BatteryIcon: width calculation and low-battery CSS class - DashboardMetric: value fallback and icon rendering logic - AppDialog: confirm/close behavior and event dispatching - PerformanceDashboard: factory function and grid class logic - DashboardToolbar: renderOptionalButtons for upload/shutdown/kiosk - SettingsDialog: isFormValid with slot counts and adjacent duplicates - DashboardForceCurve: mode cycling, division lines, shouldUpdate - App (index.ts): theme application, state deep copy, metric validation - createApp (lib/app.ts): WebSocket command dispatching, field reset Total client test count: 140 (95 existing + 45 new)
Technically I dont think there is an "errors as warning" option. Its like when you have a type error in a typed language it does not "compile". Vite allows these to be swallowed but that is like everything or nothing. I will do a bit of research but I dont have high hopes here :D The way I imagined this is that we go with a minimal viable product now, and then I can do a set of PR-s in between the feature developments where we enable stricter rules one by one. I mean there is overlap between eslint rules. On a second thought, I will check how many errors pop up if we go all in.
Which PR are you referring to? if the linux dev env then this is independent i.e. this does not need that. This is focused only on client side code. If you refer to the workout setting, well that is indeed affecting this. But Its ok I can merge/rebase unless its like a huge change. This was built on latest main, but I will rebase it on 0.9.8 np.
So this only affects the client side code. Technically, we can set the level of the depth of the typing. I mean we can set Typescript is pretty good at inferring types from vanilla js in general and you can get away by not specifying type. But the benefit in that you declare and interface is that you need to adhere that interface (e.g. you can make sure the object is never null/undefined and you can drop the null check and the constant defensive coding). Same as in any other typed language. But it can for instance create a union of strings so actual code does not need to be changed (assuming the code is correct and type safely accessed) only the interface should be declared. I would do the typing also in steps along with making the typechecker rules stricter basically. |
98c857a to
cb63b65
Compare
Client: TypeScript Migration & Vite Build Tooling
Overview
This PR modernises the entire client-side application by migrating from JavaScript to TypeScript, replacing the Rollup/Babel/esbuild build pipeline with Vite, and adding a comprehensive Vitest-based test suite.
Using this new modern workflow makes client development more efficient as Vite supports auto rebuild, browsers refresh and generally lightning fast. It integrates with Vitest seamlessly providing better testing capabilities.
Changes
Build Tooling (Rollup → Vite)
rollup.config.jsand all Rollup/Babel/esbuild dependencies with a Vite build pipeline (vite.config.js).babel.config.jsonand the oldjsconfig.json; added tsconfig.json for TypeScript compilation.icon.png,manifest.json) to public per Vite conventions.TypeScript Conversion
.jsto.ts: all components (AppDialog,AppElement,BatteryIcon,DashboardForceCurve,DashboardMetric,DashboardToolbar,PerformanceDashboard,SettingsDialog,WorkoutDialog), lib modules (app,helper,icons), stores (appState,dashboardMetrics), and the entry point (index).Record<string, any>usage throughout the client.Dependencies
nosleep.jswith@zakj/no-sleep(a maintained, typed alternative)..eslintrc.jsonwith a unified eslint.config.js at the workspace root; removedapp/client/.eslintrc.json.Test Coverage
helper.test.jswith an expandedhelper.test.ts.AppDialog,AppElement,BatteryIcon,DashboardForceCurve,DashboardMetric,DashboardToolbar,PerformanceDashboard,SettingsDialog,WorkoutDialog, app,helper,workout-utils,dashboardMetrics, andindex.Stats
44 files changed · ~3 100 insertions · ~3 700 deletions (net reduction of ~600 lines, largely from leaner auto-generated lock file and removed build configs)