Vue 3 upgrade #551
Vue 3 upgrade #551
Conversation
* 508 Vue upgrade initial change
523 - Join us component vuetify migration fixes
* 528 - mobile view scrolling * 528 - additional mobile navigation drawer fixes * 528 scrolling desktop header fix
* router fix * 539 - login form visual fix * 539 - alerts in login component fix
There was a problem hiding this comment.
I went mostly through config files and I have more questions than suggestions. However, this is because I obviously need to learn more to be able to contribute again. The amount of work done is nevertheless notable on the first glance so congratulations on handling something like that.
I don't want to hold this PR back until I will be able to review this more properly so I suggest we move onward and I'll catch up retroactively.
However there is one thing that fails at the moment. I wasn't able to log in after populating database. I got POST err everytime I tried one of the users and default password: pass123. Before we go on, probably it would be good to check if that's my bad or is it sth with the code on that side of the page.
That being said landing page works like a gold after manual check.
| @@ -14,7 +14,7 @@ | |||
| "prettier/prettier": "error", | |||
| "no-unused-vars": ["warn", { "argsIgnorePattern": "^_" }], | |||
| "no-multi-spaces": ["error", { "ignoreEOLComments": false }], | |||
| "import/no-extraneous-dependencies": ["error"], | |||
| "import/no-extraneous-dependencies": ["error", {"devDependencies": true}], | |||
There was a problem hiding this comment.
Is it a workaround? Not sure how it works exactly but it seems that true is made to ignore this rule [docs here].
There was a problem hiding this comment.
From the documentation you linked, the value Defaults to true It means that there is no change in the logic. I don't remember adding that flag manually (although it is very possible that I forgot that I did it :) ) It is possible that it was added automatically when updating some package.
| @@ -1,4 +1,4 @@ | |||
| FROM node:12 | |||
| FROM node:19 | |||
frontend/jest.config.js
Outdated
| @@ -4,5 +4,5 @@ module.exports = { | |||
| testMatch: ['**/?(*.)+(spec).js'], | |||
| collectCoverage: true, | |||
| collectCoverageFrom: ['src/**/*', '!src/assets/**'], | |||
| transformIgnorePatterns: ['/node_modules/(?!(vue-scrollactive)/)'] | |||
| transformIgnorePatterns: ['/node_modules/(?!(vue-scrollactive)/)'], | |||
There was a problem hiding this comment.
Is there a reason why we are ignoring this? I'm just curious - if the answer is more than 30sec of writing please ignore this :D
There was a problem hiding this comment.
I don't know. But because vue-scrollactive supported was dropped, I ended up rewriting scroll logic to vanilla js. In the newest PR I removed vue-scrollactive, to avoid confusion. Thank you for pointing that out.
| import { mount as nativeMount, createLocalVue } from '@vue/test-utils'; | ||
| import Vuetify from 'vuetify'; | ||
| import { mount as nativeMount } from '@vue/test-utils'; | ||
| import { vi } from 'vitest'; |
There was a problem hiding this comment.
Do I understand correctly that we are nesting Vitest in Jest? If that's correct (just to get up to speed) why we are doing this? Again - if the answer demands too much let's skip it. I'm just fishing if there's something that should be added in the docs to explain how it works.
There was a problem hiding this comment.
We are not nesting it, but unfortunately we are using both jest and vitest for now. At some point we need to reconfigure eslint to use vitest, and move all subpackages that are dependent on jest to something different (and most likely reconfigure that too).
Right now we run unittest with vitest, but eslint is configure with jest, as well as some subpackages.
Removing jest is something we should most likely do, but at lest for me it is not a simple task.
| import volontuloImg from '../assets/images/volontulo.png'; | ||
| import pahImg from '../assets/images/PAH.png'; | ||
| import alinkaImg from '../assets/images/alinka.png'; | ||
| import bankEmpatiiImg from '../assets/images/bank_empatii.png'; | ||
| import streetmixImg from '../assets/images/streetmix.png'; | ||
| import wysadzUliceImg from '../assets/images/wysadz_ulice.png'; | ||
| import stronaSpolecznosciImg from '../assets/images/StronaSpolecznosci.png'; | ||
| import watchDogImg from '../assets/images/watchdog.png'; |
There was a problem hiding this comment.
That seems so much smarter than what I have done with this before :D
@OtisRed are you able to reach backend endpoints with postman? From the error message, it seems that the backend is not running or it is misconfigured. |
Story / Bug id:
549