Repository navigation
Conversation
…...) and client build tooling (vite 8, vitest 5)
…ss 5 double-slash API URLs
…m install; node 24 for Dockerfiles, CI and client engines
…icker field styles
|
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
make one big deps bump please and make sure that the service builds into our dockerized coder worksapce
Context: project is hackforla/VRMS (PRs target
development; the captain is an admin there). It is a yarn repo with a root package.json plusclient/andbackend/packages (each with its own yarn.lock), Dockerfiles per tier (client/Dockerfile.{dev,prod,client}, backend/Dockerfile.{api,dev,prod}) and a root docker-compose.yml. The client declaresengines.node: "<=18.0.0". Recent work on development already moved Docker builds to Node 24 and the repo root as build context (#2216, #2219). The captain's dockerized Coder workspaces run on a Coder server at http://lnx.hippo-tilapia.ts.net:7080 using the Docker template (base image codercom/enterprise-base:ubuntu).What Changed
client/andbackend/packages, and regenerated all threeyarn.lockfiles.@mui/x-date-pickers9,react-router-dom7, d3 7, Vite 8 and Vitest 5. The unused@vitejs/plugin-reactand@babel/plugin-transform-react-jsx-selfwere removed.@slack/bolt5,googleapis185, Helmet 8,jsonwebtoken9,dotenv/dotenv-expand,node-cron4,nodemailer10 and others.concurrently10 and others. The old Cypress config files (cypress.jsonandcypress/plugins/index.js) were replaced withcypress.config.mjs, andbiome.jsonwas updated to the v2 schema.createRootand usesRoutes/element/Navigatein place ofSwitch/component/Redirect. Components were updated for MUI 9 (Grid, icon names,slotProps, system props, and DatePicker field styles), and the API services no longer build double-slash URLs (baseUrl + '/x') that Express 5 rejects.extendedquery parser so bracket-style queries likedate[$gt]keep working, and the catch-all 404 route is now/{*splat}. A new integration test covers thedate[$gt]events query.client/Dockerfile.dev,client/Dockerfile.prod,backend/Dockerfile.devand the frontend deploy workflow now use Node 24. The client'sengines.nodeis now>=24, and the stale Node 16 troubleshooting tip was removed fromCONTRIBUTING.md.🤖 Generated with Claude Code
Risk Assessment
🚨 High: This one PR bumps many runtime majors across both tiers (Express 5, Mongoose 9, React 19, MUI 9, react-router 7, d3 7, node-fetch 3, node-cron 4), and it already contains one confirmed silent regression: Express 5's simple query parser makes the event list on the project management page come back empty. It needs human approval and real runtime checks, not just a build.
Testing
All scenarios passed live. I built the backend and client images from the repo's docker-compose.yml on Node 24, along with the client and API production targets, then brought up the dev stack (backend, client and a throwaway mongo) under docker compose. Against that stack I checked the API with curl and the UI with Playwright, taking screenshots. The backend is on Express 5 and mongoose 9. A
date[$gt]query returns only future events, so the extended query parser is back. Unknown paths return 404 through the new/{*splat}route, and the header guard still returns 401. The client is on React 19, react-router 7 and MUI 9. Home, login, new and returning pages render, and an unknown route redirects to /. On the new-user check-in form, the MUI 9 Month/Year DatePicker keeps its old look (red rgb(250,17,79), weight 600, 40px tall), and choosing March 2023 works and enables CHECK IN. The new date[$gt] integration test passes inside the built container. I had to fix three setup problems along the way: the Docker daemon is remote, so bind mounts were empty and I baked the source into the images instead; the client hardcodes a request header that has to be quoted in .env; and mongodb-memory-server has no aarch64 debian12 binary, so I pointed it at the ubuntu-22.04 one. Everything was torn down afterwards: containers, volumes, images, the ssh tunnel and the temporary .env files.docker compose buildfrom the repo root and the backend and client dev images build on Node 24 with the bumped lockfiles/{*splat}404 fallback, and requests without the custom header are still rejectedEvidence: docker compose build log (backend + client, Node 24)
Evidence: Client production image build (vite 8 build)
Evidence: client/Dockerfile.prod build
~/.no-mistakes/evidence/01M4SH8X5ZGVJ4KZE599604F8S/api-prod-build.log)Evidence: Live API transcript (date[$gt]/[$lt] filters, 404 splat, header guard)
GET /api/events/?date[$gt]=2025-01-01 -> [Future Event, Ready Event] GET /api/events/?date[$lt]=2025-01-01 -> [Past Event] GET /api/does-not-exist -> HTTP 404 GET /api/events/ (no header) -> HTTP 401Evidence: Installed versions in running containers
~/.no-mistakes/evidence/01M4SH8X5ZGVJ4KZE599604F8S/event-integration-test.txt)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
backend/app.js:55- Express 5 switched the defaultquery parserfrom 'extended' (qs) to 'simple' (querystring), and this change doesn't set it back. The client still sends bracket-style operator queries:client/src/api/EventsApiService.js:18builds/api/events/?date[$gt]=<iso>, which ManageProjects.jsx:61 uses throughEventsApiService.fetchEvents(). Under Express 4,req.querywas{ date: { $gt: '...' } }. Under Express 5 it is{ 'date[$gt]': '...' }, andEventController.event_list(backend/controllers/event.controller.js:6-9) passes that straight toEvent.find(query). Mongoose's default strictQuery is false, so it queries a field literally nameddate[$gt]and returns[]without any error. The project management page then silently shows no events. Every other handler that handsreq.querytofindloses nested/bracket query support the same way: backend/controllers/user.controller.js:22-29 (User.find(query)) and backend/controllers/project.controller.js:7-10 (Project.find(query)). Fix: right afterconst app = express();, addapp.set('query parser', 'extended')(andqsif needed). That restores the Express 4 behavior at the one shared boundary every caller goes through. Alternatively, parse thedatefilter explicitly in event_list.client/src/components/presentational/newUserForm.jsx:218- After the move to @mui/x-date-pickers 9, the DatePicker uses the accessible field DOM structure by default (enableAccessibleFieldDOMStructureis true from v8 onward). The visible field is now made of section spans, not a text<input>. So thesxselector'& input[type=text]'(height 40px, redrgb(250, 17, 79), semi-bold, no bottom border) no longer matches anything. The Month/Year picker on the new-user check-in form loses its custom styling without any error. Fix: passenableAccessibleFieldDOMStructure={false}to keep the old input-based field, or retarget the styles to.MuiPickersSectionList-root/.MuiPickersInputBase-root.docker-compose.yml:13- This one matters for the intent ('make sure that the service builds into our dockerized coder workspace'). docker-compose mounts named volumesbackend_node_modulesandclient_node_modulesover/srv/*/node_modules. Docker only fills a named volume from the image when the volume is empty. An existing Coder workspace that runsdocker compose up --buildwill rebuild the images but keep the old Express 4 / React 18 / react-router 5 node_modules. The migrated code (Routes/Navigate, createRoot,/{*splat}) would then fail at runtime. The compose file itself is pre-existing and unchanged. Existing workspaces needdocker compose down -v(or the volumes removed) once after this bump, so this should be called out in the PR or upgrade notes.🔧 Fix applied.
1 info still open:
docker-compose.yml:13- This one matters for the intent ('make sure that the service builds into our dockerized coder workspace'). docker-compose mounts named volumesbackend_node_modulesandclient_node_modulesover/srv/*/node_modules. Docker only fills a named volume from the image when the volume is empty. An existing Coder workspace that runsdocker compose up --buildwill rebuild the images but keep the old Express 4 / React 18 / react-router 5 node_modules. The migrated code (Routes/Navigate, createRoot,/{*splat}) would then fail at runtime. The compose file itself is pre-existing and unchanged. Existing workspaces needdocker compose down -v(or the volumes removed) once after this bump, so this should be called out in the PR or upgrade notes.✅ **Test** - passed
✅ No issues found.
docker compose buildfrom the repo root and the backend and client dev images build on Node 24 with the bumped lockfiles/{*splat}404 fallback, and requests without the custom header are still rejecteddocker compose -p nmvrmstest build backend client(repo docker-compose.yml, root build context, Node 24)docker build -f client/Dockerfile.client --target client-production .(vite 8 production build)docker build -f client/Dockerfile.prod --target client-production .docker build -f backend/Dockerfile.api --target api-production .docker compose up mongo backend clientwith a disposable override: mongo:7 service, source baked into images because the daemon is remote (ssh://mini2), throwaway .env filescurl against the running backend: GET /api/events (unfiltered,date[$gt],date[$lt], checkInReady), unknown /api and deep paths for the 404 fallback, the missing-header guard, /api/healthcheck, /api/projectsPlaywright (in a container sharing the client's network) loading /, /login, /new, /returning, an unknown route, and /checkIn/newUser with screenshotsPlaywright DatePicker flow: choose 'No' on the first-time question, check computed styles of .MuiPickersInputBase-sectionsContainer, open the picker, pick 2023 then March, confirm the CHECK IN button is enablednpx vitest run test/event.integration.test.jsinside the built backend container (MONGOMS_DISTRO=ubuntu-22.04)Recorded installed dependency versions inside the running containers✅ **Document** - passed
✅ No issues found.
backend/app.js:3- Biome 2.5.15 still reports error-level diagnostics in files this change touched, and none of them come from this change. (1) assist/source/organizeImports in backend/app.js and backend/env.bootstrap.js. The import order there is old, and in app.js the side-effect imports are mixed in with statements (assertEnv, workers, cleanupExpiredTokens), so sorting automatically could change load order. (2) lint/suspicious/noArrayIndexKey in client/src/components/manageProjects/editPMs/projectMembersList.jsx:178 and client/src/components/user-admin/UserPermissionSearch.jsx:38,76. Fixing these needs stable keys, which is a behavior change. These were left as they are: the repo's lint-staged hook runsbiome check --write ... || true, so it does not block commits on them.✅ **Push** - passed
✅ No issues found.