♻️ Refactor frontend

[bosdch1]

Hi,

thanks for the great work on FastAPI and this template. We are using it in my team at Hella Aglaia Mobile Vision GmbH to build an internal tool for our machine learning services.
While working with the frontend template, I have noticed that some of the dependencies are outdated, so I started refactoring the frontend to bring everything up-to-date. Here's what I did.

• ♻️ Migrate to latest Vuetify 2.x API, for details see migration guide (https://vuetifyjs.com/en/getting-started/releases-and-migrations/#migration-guide)
• ♻️ Migrate to latest Vee-Validate 3.x API, for details see migration guide (https://logaretm.github.io/vee-validate/migration.html#migration-guide)
• ➕ Refactor state management with vuex-module-decorators (https://github.com/championswimmer/vuex-module-decorators), which is similar in style to vue-class-components
• ✨ Add configuration for eslint (+prettier, +typescript)
• 🎨 Reformat code with eslint --fix
• 📌 Upgrade/add relevant dependencies
• 🚚 Remove external dependencies (e.g. mdi fonts) by adding them as direct dependencies

Maybe there is interest in these changes.

Cheers

[Nonameentered]

I'm still learning frontend development at the moment, could you or someone else explain why the test is failing?

If we switch to eslint, do we still need tslint.json?

When I do npm run lint I get errors like these in shims-tsx.d.ts although I don't have any issues when running npm run serve:

error: 'Vue' is defined but never used. Allowed unused vars must match /^_/u (@typescript-eslint/no-unused-vars) at src/shims-tsx.d.ts:1:8:
> 1 | import Vue, { VNode } from "vue";
    |        ^
  2 | 
  3 | declare global {
  4 |   namespace JSX {


error: 'VNode' is defined but never used. Allowed unused vars must match /^_/u (@typescript-eslint/no-unused-vars) at src/shims-tsx.d.ts:1:15:
> 1 | import Vue, { VNode } from "vue";
    |               ^
  2 | 
  3 | declare global {
  4 |   namespace JSX {


warning: Interface name must be prefixed with "I" (@typescript-eslint/interface-name-prefix) at src/shims-tsx.d.ts:6:15:
  4 |   namespace JSX {
  5 |     // tslint:disable no-empty-interface
> 6 |     interface Element extends VNode {}
    |               ^
  7 |     // tslint:disable no-empty-interface
  8 |     interface ElementClass extends Vue {}
  9 |     interface IntrinsicElements {

Should this be happening?

[Nonameentered]

This fixes the errors with testing.
To remove the warning Unknown custom element which occured after fixing the test, I also edited upload-button.spec.ts to include the following after import "@/plugins/vuetify";:

import Vue from 'vue'
import Vuetify from 'vuetify'

Vue.use(Vuetify)

Not sure how to/if it's possible to suggest these changes since it wasn't already edited in the PR.

[bosdch1]

Hi @Nonameentered,

thank you very much for pointing out these issues! I too have just started front-end development, so I really appreciate someone peer-reviewing my changes.

If we switch to eslint, do we still need tslint.json?

You are right, tslint.json is obsolete if we switch to eslint. I missed that!

When I do npm run lint I get errors like these in shims-tsx.d.ts although I don't have any issues when running npm run serve:

[...]

Should this be happening?

These shims are auto-generated by vue-cli and I think they can safely be ignored by eslint. I will add the corresponding configuration.

Regarding the changes you suggested, they all look valid. I will incorporate them into the pull request :)

Thanks again!

[bosdch1]

Not sure how to/if it's possible to suggest these changes since it wasn't already edited in the PR.

I think one way could be to do a Pull Request on the branch inside my fork, but not sure.

[Nonameentered]

Looks like the Manage Users page is broken in that you can't click to edit the user through the actions column.

[Nonameentered]

Fixes Manage Users screen to get is_active, is_superuser, and actions working

[bosdch1]

Ah yes, I've been meaning to address this issue, but hadn't found time to do so yet.

Naming conventions in JavaScript mostly require camelCase variables, whereas in Python we like snake_case. This will of course trigger a linter on either side, depending which style you choose. In this case, we choose snake_case which will satisfy flake8 (with plugin pep8-naming) but trigger eslint (as configured in this pull request).
However, there is a workaround for this, which will satisfy both: Pydantic models can be configured with an alias_generator that takes a function which converts all field names (e.g. snake to camel case) which can be used during serialization (model.json(by_alias=True)). See an example implementation of this here: https://github.com/nsidnev/fastapi-realworld-example-app/blob/master/app/models/domain/rwmodel.py
This would make all schemas compatible with PEP8 in the backend and eslint in the frontend.

For now though, your changes are correct.

[monatis]

Hi @tiangolo, any plan to merge this pr? Or should we add it to our fork?

[monatis]

I forked @cbhagl's refactor-frontend branch and then pulled @tiangolo's master and pushed it into my fork. Anyone who wants to start with Vuetify 2 can use this one until this PR is merged.

[Koschi13]

@Nonameentered why did you used ~ for some dependencies? Isn't ^ better, as this would pull new features without breaking anything? Or am I wrong here?

[bosdch1]

I think vue-cli deliberately uses ~ for the plugins it manages.

[Koschi13]

Ok, maybe I mentioned the wrong user ^^ Anyway I got the right person, why don't we replace the ~ with ^. I don't know JS that much but it seems to be better to use ^ for the dependencies as we could only benefit from the new minor versions.
Also this project serves as a template, therefore it would be more future proof to use ^ instead of ~.

[bosdch1]

I think vue-cli wants you to use vue upgrade to upgrade its plugins because those upgrades might involve some changes that need the users attention.

The safest approach I think is just to upgrade vue-cli every once in a while and update the template.

[Koschi13]

@cbhagl didn't know that, thanks for the clarification.

[Koschi13]

My editor (VS-Code) gives me the following error: 'v-slot' directive doesn't support any modifier. I googled this and found the following answer: https://stackoverflow.com/a/63309643
Which solved this "issue" for me.

Except of this project I never programmed in JS, therefore I don't know the exact reason and if this solution is appropriate. However, I changed my code to look like this (I hate red warnings 😛):

    <v-data-table :headers="headers" :items="users">
      <template v-slot:[`item.is_active`]="{ item }">
        <v-icon v-if="item.is_active">mdi-check</v-icon>
      </template>

      <template v-slot:[`item.is_superuser`]="{ item }">
        <v-icon v-if="item.is_superuser">mdi-check</v-icon>
      </template>

      <template v-slot:[`item.actions`]="{ item }">
        <v-icon @click="routeEditUser(item.id)">mdi-pencil</v-icon>
      </template>

[bosdch1]

What version of eslint are you using?
VScode probably complains because either the language server is unhappy or a linter emits a warning. In this case I think it is the latter (eslint) and the warning you are describing must be a new default, because I did not see this before.
From my understanding of Vue this is indeed valid syntax and would fix the warning. However, I think it is kind of ugly in this case and maybe suppressing the warning (either via inline comment or global setting) would be better?

[Koschi13]

My version is v6.8.0 and indeed, the "correct" syntax isn't that beautiful. I'll provide a request change with the corresponding ignore comment.

[Koschi13]

Somehow adding <!-- eslint-disable-next-line vue/valid-v-slot --> does not help here... I don't know what I'm missing here.

[bosdch1]

Seems like others run into this problem as well: vuejs/eslint-plugin-vue#260 (comment)

[bosdch1]

Have you tried <!-- eslint-disable-next-line eslint-plugin-vue/valid-v-slot --> or <!-- eslint-disable-next-line @vue/valid-v-slot -->?

EDIT: Just realized that this doesn't make sense and shouldn't work...

[Koschi13]

Ok, I checked if a update of everything to the latest version fixes this and it did not. But I also opened the project in PyCharm Professional and the error was not linted at all. So it seems to me, that this is a problem of VS-Code? Anyway, as this problem is not present in all editors I wouldn't change it.

[bosdch1]

Try running eslint directly on the file (via command-line) and see if the error appears. Correctly configured editors will generally output the same warnings as they are just displaying what either the vue language server or (in our case) eslint say. I think the error you are seeing is some new default rule (ie. vue/valid-v-slot) that eslint-plugin-vue complains about and there is an bug in the plugin that prohibits correctly disabling it.

[bosdch1]

Build fails because response is now unused.

[bosdch1]

Fix build errors.

[KevinGrey-sys]

I refactored my project by hand using your code here as a guide and it worked splendidly.

Excellent work! Thank you.

[tiangolo]

Thanks for all the work here @cbhagl! 🙇

I wanted to refactor the frontend and move to React with TypeScript and hooks, etc. It's now been done in other PRs, so I'll pass on this one. But thanks for all the effort! 🍰