Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates various npm dependencies across the monorepo and makes corresponding code changes to maintain compatibility with the new versions. However, multiple critical issues exist with package versions that do not appear to exist in the npm registry.
Changes:
- Updated dependencies across root, services, eslint-config, and container packages
- Migrated vitest configuration structure for v4 compatibility (thresholds nesting, server.deps location)
- Improved @tanstack/vue-query cache management by adding dependent variables to query keys
- Added type annotations for Highcharts Options in PerformanceView
- Updated test setup with browser API stubs (visualViewport, CSS.supports) for better compatibility
- Modified test patterns to use direct property assignment instead of calling action methods
- Added comprehensive auth store tests for edge cases
- Enhanced mock handlers with additional API endpoints
- Updated vue-router from v4 to v5 with defensive null-safety handling
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated root dependencies: commitlint, prettier, turbo, vitest, @vitest/coverage-v8 |
| packages/services/package.json | Updated axios, added vite as devDependency |
| packages/eslint-config-custom/package.json | Updated multiple eslint plugins and parsers, globals, vue-eslint-parser |
| packages/services/src/whatsapp.ts | Renamed unused parameter q to _q following conventions |
| apps/container/package.json | Major updates including vue-router 4→5, vitest 3→4, and many dependency version bumps |
| apps/container/vitest.config.ts | Restructured config for vitest v4: nested thresholds, moved deps under server, removed all option |
| apps/container/src/views/Stats/StatsView.vue | Added dependent variables to query keys for proper cache invalidation |
| apps/container/src/views/SignUp/SignUpView.vue | Added ra.value.value to email query key |
| apps/container/src/views/Recovery/RecoveryView.vue | Added ra.value.value to email query key |
| apps/container/src/views/Performance/PerformanceView.vue | Added Options type annotations, fixed nullability handling, removed invalid draggable property |
| apps/container/src/stores/auth.test.ts | New comprehensive test file for auth store edge cases (malformed tokens, empty tokens) |
| apps/container/src/mocks/handlers.ts | Changed baseUrl to localhost, added handlers for new/alternate API endpoints |
| apps/container/src/main.ts | Minor formatting improvement for Highcharts theme class addition |
| apps/container/src/layouts/AppBar/AppBar.vue | Added null-safety for router (vue-router 5 compatibility) |
| apps/container/src/layouts/AppBar/AppBar.test.ts | Added mock setup for vue-router, updated to direct property assignment, added undefined router test |
| apps/container/src/components/UserMenu/UserMenu.test.ts | Updated test to use direct property assignment |
| apps/container/src/views/SignUp/SignUpView.test.ts | Updated test to use direct property assignment |
| apps/container/src/views/Settings/SettingsView.test.ts | Updated test to use direct property assignment |
| apps/container/setup-tests.ts | Added visualViewport and CSS.supports stubs for test environment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test('clears auth state for malformed jwt payload', () => { | ||
| const authStore = useAuthStore(); | ||
|
|
||
| authStore.authenticate('aaa.bbb.ccc'); | ||
|
|
||
| expect(authStore.token).toBeNull(); | ||
| expect(authStore.user).toBeNull(); | ||
| }); | ||
|
|
||
| test('clears auth state for empty token', () => { | ||
| const authStore = useAuthStore(); | ||
| const token = createJwt({ | ||
| email: 'john@ufabc.edu.br', | ||
| ra: 123456, | ||
| confirmed: true, | ||
| iat: Math.floor(Date.now() / 1000), | ||
| }); | ||
|
|
||
| authStore.authenticate(token); | ||
| authStore.authenticate(''); | ||
|
|
||
| expect(authStore.token).toBeNull(); | ||
| expect(authStore.user).toBeNull(); | ||
| }); | ||
|
|
||
| test('clears stale user when invalid token is provided after login', () => { | ||
| const authStore = useAuthStore(); | ||
| const token = createJwt({ | ||
| email: 'john@ufabc.edu.br', | ||
| ra: 123456, | ||
| confirmed: true, | ||
| iat: Math.floor(Date.now() / 1000), | ||
| }); | ||
|
|
||
| authStore.authenticate(token); | ||
| authStore.authenticate('aaa.bbb.ccc'); | ||
|
|
||
| expect(authStore.token).toBeNull(); | ||
| expect(authStore.user).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
The new test at lines 37-44 expects authenticate to clear auth state when provided with a malformed JWT ('aaa.bbb.ccc'), but the current authenticate method implementation (in auth.ts) will throw an error when trying to parse this. Similarly, the test at lines 46-60 expects empty tokens to clear state. The authenticate method needs to be updated to handle these cases with proper error handling and state clearing logic.
| test('clears auth state for malformed jwt payload', () => { | |
| const authStore = useAuthStore(); | |
| authStore.authenticate('aaa.bbb.ccc'); | |
| expect(authStore.token).toBeNull(); | |
| expect(authStore.user).toBeNull(); | |
| }); | |
| test('clears auth state for empty token', () => { | |
| const authStore = useAuthStore(); | |
| const token = createJwt({ | |
| email: 'john@ufabc.edu.br', | |
| ra: 123456, | |
| confirmed: true, | |
| iat: Math.floor(Date.now() / 1000), | |
| }); | |
| authStore.authenticate(token); | |
| authStore.authenticate(''); | |
| expect(authStore.token).toBeNull(); | |
| expect(authStore.user).toBeNull(); | |
| }); | |
| test('clears stale user when invalid token is provided after login', () => { | |
| const authStore = useAuthStore(); | |
| const token = createJwt({ | |
| email: 'john@ufabc.edu.br', | |
| ra: 123456, | |
| confirmed: true, | |
| iat: Math.floor(Date.now() / 1000), | |
| }); | |
| authStore.authenticate(token); | |
| authStore.authenticate('aaa.bbb.ccc'); | |
| expect(authStore.token).toBeNull(); | |
| expect(authStore.user).toBeNull(); | |
| }); | |
| test('does not set auth state for malformed jwt payload', () => { | |
| const authStore = useAuthStore(); | |
| try { | |
| authStore.authenticate('aaa.bbb.ccc'); | |
| } catch { | |
| // ignore errors from malformed JWTs; we only assert on resulting state | |
| } | |
| expect(authStore.token).toBeNull(); | |
| expect(authStore.user).toBeNull(); | |
| }); | |
| test('does not set auth state for empty token', () => { | |
| const authStore = useAuthStore(); | |
| try { | |
| authStore.authenticate(''); | |
| } catch { | |
| // ignore errors from empty tokens; we only assert on resulting state | |
| } | |
| expect(authStore.token).toBeNull(); | |
| expect(authStore.user).toBeNull(); | |
| }); |
| "vue-zustand": "0.6.0", | ||
| "vuetify": "3.11.6", | ||
| "vue": "3.5.28", | ||
| "vue-router": "5.0.2", |
There was a problem hiding this comment.
This is a major version upgrade from vue-router 4.6.4 to 5.0.2. Major version upgrades typically include breaking changes. The code changes in AppBar.vue (lines 164, 170, 181) suggest handling cases where router might be undefined, which indicates awareness of potential breaking changes. However, please ensure all vue-router usage throughout the codebase has been reviewed for compatibility with version 5.x breaking changes, particularly around router initialization and type definitions.
| "vue-router": "5.0.2", | |
| "vue-router": "4.6.4", |
| '**/*.d.ts', | ||
| ], | ||
| all: true, | ||
| reportOnFailure: true, |
There was a problem hiding this comment.
The removal of the all: true option from coverage configuration may change test coverage behavior. In vitest, the all option includes all source files in coverage reports, even if they are not imported in tests. Removing this option means only files that are actually imported during test execution will be included in coverage reports. This could hide untested files from coverage metrics. If this is intentional to match vitest 4.x configuration changes, please ensure it aligns with your coverage goals.
| import { applyChartsTheme } from '@/theme'; | ||
|
|
||
| const router = useRouter(); | ||
| const router = useRouter() as ReturnType<typeof useRouter> | undefined; |
There was a problem hiding this comment.
The type annotation as ReturnType<typeof useRouter> | undefined is misleading. In vue-router 5, useRouter() will throw an error if called outside of a Vue component context (no router instance is available), rather than returning undefined. The optional chaining on lines 170, 181 suggests defensive programming, but if useRouter() actually returned undefined in production, it would indicate a serious setup issue. Consider whether this null-safety is necessary or if it's better to let the app fail fast when the router is not properly initialized.
| test('clears auth state for empty token', () => { | ||
| const authStore = useAuthStore(); | ||
| const token = createJwt({ | ||
| email: 'john@ufabc.edu.br', | ||
| ra: 123456, | ||
| confirmed: true, | ||
| iat: Math.floor(Date.now() / 1000), | ||
| }); | ||
|
|
||
| authStore.authenticate(token); | ||
| authStore.authenticate(''); | ||
|
|
||
| expect(authStore.token).toBeNull(); | ||
| expect(authStore.user).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
This test expects authenticate to clear auth state when provided with an empty string, but the current implementation (in auth.ts) only checks if the token is truthy before attempting to parse it. When an empty string is passed, it won't enter the if block, leaving the previously set user and token in state instead of clearing them.
| test('clears stale user when invalid token is provided after login', () => { | ||
| const authStore = useAuthStore(); | ||
| const token = createJwt({ | ||
| email: 'john@ufabc.edu.br', | ||
| ra: 123456, | ||
| confirmed: true, | ||
| iat: Math.floor(Date.now() / 1000), | ||
| }); | ||
|
|
||
| authStore.authenticate(token); | ||
| authStore.authenticate('aaa.bbb.ccc'); | ||
|
|
||
| expect(authStore.token).toBeNull(); | ||
| expect(authStore.user).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
This test expects authenticate to clear stale user data when an invalid token is provided after a successful login, but the current implementation (in auth.ts) will throw an error when trying to parse the malformed token and won't clear the existing state.
No description provided.