Repository navigation
feat: React 19 / MUI 9 for the web interface, and a home screen that lists the apps - #463
Conversation
React 17 and MUI 5 are out of support; this moves the interface to React 19,
MUI 9 and react-router 7, and brings the build toolchain up with them.
Code changes the new majors require:
- ReactDOM.render -> createRoot (index.tsx)
- the global JSX namespace is gone in React 19: JSX.Element -> React.JSX.Element
- makeStyles is gone from MUI: the one usage (the Studio log container) moves
into the CSS module that file already imports
- Grid's `item` and breakpoint props -> `size={{ ... }}`, and `direction` is no
longer a Grid prop
- Snackbar's TransitionComponent -> slots.transition
- Typography's m/p shorthand props -> sx
- TabList's TabIndicatorProps -> slotProps.indicator
- ErrorOutline -> ErrorOutlined (icons renamed)
- toast.POSITION.BOTTOM_RIGHT -> the 'bottom-right' string (react-toastify 11)
- @babel/plugin-proposal-class-properties is part of preset-env in Babel 8
Removed, all dead CRA-era leftovers: eslint, eslint-config-react-app,
eslint-webpack-plugin (no webpack config imports the plugin and no script runs
eslint - the lint script is prettier), the eslintConfig block in package.json,
setupTests.ts and react-app-env.d.ts (no test runner in web), @testing-library/*
and @types/react-router-dom.
The webpack configs are loaded by webpack-cli 7 through Node's own TypeScript
support, so they take explicit .ts extensions, `import type` for type-only
imports, and import.meta.dirname in place of __dirname.
npm audit --audit-level=high in web drops from 20 (18 high) to 5, all one
advisory, via the dev server's proxy library.
Verified: tsc clean, webpack builds, the web test scripts pass, and the running
interface renders under React 19 (driven headlessly over CDP).
The single-page app's home screen needs the list of streamed apps. It was only rendered into the landing page's HTML, so there was nothing to read. /appStream/apps.json answers with the same permission-filtered config the landing page uses - the same function, not a copy of the rule - so the list the home screen shows and the list the page shows cannot disagree. It is declared before the `/*splat` handler, which would otherwise serve the path as an app's static asset. Covered by appStreamAppsJson.spec.ts: unauthorized, no grants, a single grant, a stream-wide grant with one app denied, an admin, and a case asserting the JSON and the HTML agree.
Desktop mode has no accounts and no permission model - one fixed user holds everything - but /SASjsApi/info/myAuthorizedRoutes looked the user up in the database and answered 404 when there was none, which is always in desktop mode. The interface reads that list to decide what to offer, so in desktop mode it offered nothing: no Studio tab, no Apps button. It now answers with every route, which is the same answer the permission gate gives.
The home screen was a welcome paragraph. It is now the list of things this caller can open: Studio and API Explorer first, then every app streamed to this deployment, each as a tile. The streamed apps come from the server already filtered to what the caller holds a grant on, so no tile is a dead end. Verified against a running server with two published apps: the home screen renders Studio, API Explorer, Mario and Sonic in that order, linking to #/SASjsStudio, /SASjsApi, /AppStream/Mario/ and /AppStream/Sonic/.
![sasjs-dev[bot]](https://avatars.githubusercontent.com/u/67858352?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the full base...head diff (29 files: the /AppStream/apps.json endpoint with a new spec, the desktop-mode authorized-routes fix, the React 19 / MUI 9 migration of the web interface, and the babel/webpack toolchain bumps), plus the JS hardening checklist, the gitleaks pre-commit hook, release-driving commit types, and the live CI status at head.
-
.github/workflows/build.yml:56 - the security job is red at this head. The API audit step (npm ci --ignore-scripts && npm audit --audit-level=low) fails with 3 high advisories: braces GHSA-vfj7-8cjw-p6xm via chokidar and nodemon. The api lockfile is untouched by this diff and main is green, so a newly published advisory hit a pre-existing dev-only chain; braces 3.0.3 is the latest published release, so npm audit fix cannot clear it. Verified locally against the head api lockfile: npm audit --omit=dev reports 0 vulnerabilities, so scoping this step to production deps (as the root audit step at build.yml:50 already does) is the least-invasive fix until nodemon moves to chokidar 4; forcing chokidar 4 via overrides is riskier because nodemon relies on its glob support.
-
.github/workflows/build.yml:66 - the Web audit step fails the same way: verified locally against the head web lockfile, npm audit --audit-level=high reports 5 high advisories (the same unpatched braces, via micromatch, http-proxy-middleware, webpack-dev-server 6, and webpack-cli 7.2.3, the last bumped in this PR). The step's comment justifies --audit-level=high by the two react-router moderates; those have cleared with this PR, so once the braces chain is handled the step should return to strict (default or --audit-level=low) instead of staying at high. npm audit --omit=dev is clean on the web workspace as well.
-
web/.nvmrc:1 - still pins v16.15.1, but the bumped toolchain needs a newer Node: sass-loader 17.0.1 engines >=22.11.0, @babel/core 8.0.6 engines ^22.18.0 || >=24.11.0, webpack-cli 7.2.3 engines >=20.9.0, and web/webpack.common.ts:9 swapped __dirname for import.meta.dirname (Node >=20.11). CI runs lts/* and the container builds on Node 24, but a contributor following web/.nvmrc gets a broken local install. Bump it to match.
-
api/src/routes/appStream/index.ts:83 and web/src/components/home.tsx:167 - the streamed-app tiles never show an app's own logo. apps.json returns the streamLogo value raw (logo: entry.streamLogo ?? null) and the tile renders it as the img src directly, so a config value like "logo.png" resolves against the SPA root (where web/public/logo.png is served), not the app's asset path; other values 404 and fall back to the default logo. The HTML landing page resolves the same value as the app name plus slash plus logo (api/src/routes/appStream/appStreamHtml.ts). Return the resolved URL from apps.json (logo: entry.streamLogo ?
/AppStream/${name}/${entry.streamLogo}: null) or prefix it in home.tsx. -
web/package.json:28 and web/package.json:38 - the dependency blocks were rewritten wholesale but keep ^ ranges on most entries even though web/.npmrc sets save-exact=true (for example "react": "^19.3.0" and "@babel/core": "^8.0.6"). The lockfile pins them regardless; pin the changed entries exactly so the manifest matches the repo's own save-exact setting.
-
api/src/controllers/info.ts:125 - the new desktop-mode early return (if (process.env.MODE === ModeType.Desktop) return { paths: getAuthorizedRoutes() }) has no test. api/src/routes/api/spec/myAuthorizedRoutes.spec.ts covers server mode only, and the branch is directly unit-testable by setting process.env.MODE before calling the controller; add a case so the desktop behaviour this PR relies on stays pinned.
6 findings above for review.
The interface's screens are real paths, so a deep link or a refresh - /SASjsStudio, /SASjsSettings - needs the shell rather than a 404. Anything that reaches the end of the middleware chain is not an API route, not an AppStream app and not a file, so it is one of the interface's routes: serve index.html and let the router resolve it. Reserved prefixes and paths that name a file are refused with a 404, so an unknown API path stays a 404 and a missing asset does not come back as HTML.
The interface used a hash router, so its screens lived at /#/SASjsStudio and the like. The hash dates from October 2021 (c72867d), when only / served the app shell and a deep link to a real path found nothing. The server answers those paths now, so that constraint is gone. Studio, Settings and the authorization code screen are the interface's own screens and are addressed as paths. Streamed apps stay under /AppStream. Two places assumed the path was always / and now say what they mean: the axios base URL is the origin rather than the origin plus the current path, and the Studio run menu opens the execute URL against the origin. The Studio route also stops treating "no answer yet" as "refused". The authorized-routes request is in flight on first paint, and redirecting on an empty list bounced a deep link to Home before the server had been asked - the context now reports whether the answer has arrived and the route waits for it. A link built the old way still works: a fragment never reaches the server, so the entry point moves a fragment-shaped route into the path before rendering. That covers existing bookmarks and the CLI's authorization code URL, which gains a path-shaped URL in its own change.
There was a problem hiding this comment.
Re-review at the new head a53a8ee, which supersedes the review at 652064b. Checked the full base...head diff, focused on the delta since the last review (the BrowserRouter migration, the server-side app-shell catch-all, the legacy hash shim, the run-menu and axios base URL fixes), plus the JS hardening checklist, the gitleaks hook, release-driving commit types, and the live CI status at head.
-
.github/workflows/build.yml:56 - the security job is red at this head. The API audit step (npm ci --ignore-scripts && npm audit --audit-level=low) fails with 3 high advisories for braces GHSA-vfj7-8cjw-p6xm via chokidar 3 / nodemon. Same failure as the previous head and the api lockfile is untouched by this diff. Verified locally against the head api lockfile: npm audit --omit=dev reports 0 vulnerabilities, so scoping this step to production deps (as the root step at build.yml:50 already does) is the least-invasive fix until nodemon moves to chokidar 4.
-
.github/workflows/build.yml:66 - the Web audit step still fails on the same unpatched braces chain (5 high advisories verified locally against the head web lockfile; the CI job aborted at the API step so this step never ran). The step's comment still justifies --audit-level=high by two react-router moderates, but this PR already bumped react-router-dom to 7.18.4 and those cleared: once the braces chain is handled, return the step to strict (default level or --audit-level=low) instead of leaving it at high.
-
api/src/app.ts:145 - the reserved-prefix check is case-sensitive while Express mounts the API router case-insensitively, so GET /sasjsapi/unknown with an Accept: text/html header falls past the API router and is served the app shell instead of the intended 404 (reproduced locally against express 5.2.1: /SASjsApi/unknown -> 404, /sasjsapi/unknown -> shell). Compare the prefixes case-insensitively, for example req.path.toLowerCase().
-
api/src/app.ts:156 - the catch-all serves the shell without minting the XSRF-TOKEN cookie that the same request gets from the web router's own shell route (api/src/routes/web/web.ts:32 calls setCSRFCookie). The client currently compensates - web/src/context/appContext.tsx:177 refreshes the cookie via axios.get('/') on the signed-out path - so today it works, but the two entry points serving the same shell now have different side effects and the deep-link flow depends on an extra round trip. Call setCSRFCookie here as well, or serve the shell through the web router so the paths cannot drift.
-
api/src/app.ts:141 - the new catch-all route has no test coverage. Its branches (non-HTML accept 404, reserved prefixes 404, file-extension 404, missing web build) are directly testable with supertest the way the rest of api/src/routes is covered; nothing under api/src/routes/**/spec exercises it.
-
web/src/App.tsx:75 - the authenticated Routes list has no path="*" fallback, and the server now answers any unknown path with the app shell (api/src/app.ts:141), so a mistyped URL returns 200 with a blank screen instead of a redirect home or a not-found screen. Add a catch-all route (Navigate to="/" or a not-found element).
-
web/.nvmrc:1 - still pins v16.15.1 while the bumped toolchain needs a newer Node: sass-loader 17.0.1 engines >=22.11.0, @babel/core 8.0.6 engines ^22.18.0 || >=24.11.0, webpack-cli 7.2.3 engines >=20.9.0, and web/webpack.common.ts:9 now uses import.meta.dirname (Node >=20.11). A contributor following .nvmrc gets a broken local install. Unchanged from the previous review.
-
api/src/routes/appStream/index.ts:83 and web/src/components/home.tsx:167 - unchanged from the previous review: apps.json returns streamLogo raw (logo: entry.streamLogo ?? null) and the tile renders it as the img src against the SPA root, so a configured logo like "logo.png" resolves to the interface's own /logo.png or 404s into the onError fallback, never the app's asset. The HTML landing page resolves the same value as streamServiceName + '/' + logo (api/src/routes/appStream/appStreamHtml.ts:18). Return the resolved /AppStream// URL from apps.json, or prefix it in home.tsx.
-
web/package.json:28 - unchanged: the rewritten dependency blocks keep ^ ranges on most entries even though web/.npmrc sets save-exact=true (for example "react": "^19.3.0" at line 28, "@babel/core": "^8.0.6" at line 38, "webpack-cli": "^7.2.3" at line 67). The lockfile pins them regardless; pin the changed manifest entries exactly so it matches the repo's own setting.
-
api/src/controllers/info.ts:125 - unchanged: the desktop-mode early return (if (process.env.MODE === ModeType.Desktop) return { paths: getAuthorizedRoutes() }) still has no test - api/src/routes/api/spec/myAuthorizedRoutes.spec.ts covers server mode only. The branch is directly unit-testable by setting process.env.MODE before calling the controller.
10 findings above for review.
Two ways the catch-all could hand back the shell where it should not. The reserved-prefix refusal compared case-sensitively while Express matches the mounted routers case-insensitively, so /saslogon/unknown fell past the web router and was served the app shell instead of a 404. The comparison is now case-insensitive. The catch-all also served the shell without the CSRF cookie the web router's own shell route sets. The client compensated by fetching / on the signed-out path, so it worked - but the two entry points had different side effects and a deep link paid an extra round trip. Both now set the cookie. The branches are covered by a new spec, which mocks the web controller so the shell path does not depend on a build being present.
The interface's screens are real paths now, so the shell is served from /SASjsStudio and from deeper paths too. Asset references were relative, which resolves against the current route: at /a/b/c the page asked for /a/b/index.bundle.js and the bundle 404'd, leaving a blank screen. The production config pinned publicPath to './' as well, so setting it in the shared config had no effect until that override went. publicPath is '/' in one place now, and the header's logo and the template's favicon and manifest are absolute like the rest. With the bundle loading at any depth, a mistyped URL reaches the router, so the authenticated routes gain a catch-all that sends it home rather than rendering an empty screen.
apps.json returned the configured logo raw, and the interface rendered it as an img src against its own root - so a value like "logo.png" resolved to the interface's logo or 404'd into the fallback, never the app's asset. The landing page resolves the same value as <app>/<logo>. The endpoint resolves it the same way now, and the spec pins the two surfaces against each other so they cannot drift.
The cached token pair can become unusable while the process keeps running: the access token expires and the refresh token that would replace it is single-use and already spent. Every later request was refused until the process restarted, and resetViyaAuth existed for this but nothing called it. A refused token is now detected, the cache dropped and the job retried once. A program failure is not a token failure and is not retried.
The desktop-mode early return decides what the interface offers in desktop mode, where an empty answer hides everything. It had no coverage.
The web audit failed on an advisory no upgrade clears: braces GHSA-vfj7-8cjw-p6xm, reached through webpack-cli -> webpack-dev-server -> http-proxy-middleware -> micromatch, and braces has published no fixed release. It is a build-time denial of service in a tool that never ships. The step covers the production set, the same scope the root audit uses. The two react-router moderates cleared with react-router-dom 7, so the level returns to the default instead of staying at high.
web/.npmrc sets save-exact, but the rewritten dependency blocks kept ^ ranges. The entries are pinned to the versions the lockfile already resolved. The three .nvmrc files pinned v16.15.1, which no longer matches the toolchain: the web build needs Node 22 or later, and the api's start script uses `node --watch`, which needs 18.11. They track lts/* now, as CI and the container already do.
|
Addressed all twelve findings from the two reviews, plus one the review surfaced that was not on the list. The two auditsThe API audit clears on its own: The web audit cannot be cleared. The remaining advisory is Findings
One more, found while verifyingThe catch-all serves the shell at any depth, but the assets were relative - at VerifiedDriven over CDP against a running server:
|
|
🎉 This PR is included in version 1.21.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
1 participant
The interface's own screens are now real paths
/#/SASjsStudiois/SASjsStudio. Studio, Settings and the authorization code screen are the interface's own screens, so they are addressed as paths; streamed apps stay under/AppStream.The hash was not a preference.
c72867dswappedBrowserRouterforHashRouterin October 2021 because only/served the app shell, so a deep link to a real path found nothing. This PR gives the server the fallback that removes that constraint, so the hash has no reason to stay.Server
Anything that reaches the end of the middleware chain is not an API route, not an AppStream app and not a file, so it is one of the interface's routes:
index.htmlis served and the router resolves it. Reserved prefixes and paths that name a file are refused with a 404, so an unknown API path stays a 404 and a missing asset does not come back as HTML.Interface
Two places assumed the path was always
/and now say what they mean: the axios base URL is the origin rather than the origin plus the current path, and the Studio run menu opens the execute URL against the origin.The Studio route also stops treating "no answer yet" as "refused". The authorized-routes request is in flight on first paint, and redirecting on an empty list bounced a deep link to Home before the server had been asked. The context now reports whether the answer has arrived, and the route waits for it.
Compatibility
A fragment never reaches the server, so the entry point moves a fragment-shaped route into the path before rendering. Existing bookmarks and the CLI's current authorization code URL (
${serverUrl}/#/SASjsLogon?...) keep working. The CLI gains a path-shaped URL in its own change.Verified
Driven over CDP against a running server with two published apps:
/SASjsStudio/SASjsStudio/#/SASjsStudio/SASjsStudio///SASjsLogon?client_id=test-client&response_type=code/SASjsLogonlocation.search, issues/refuses the code/nonexistent.jstscclean on both sides, webpack builds, prettier clean.