Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor enhancements #92

Open
sajadhsm opened this issue Feb 27, 2022 · 2 comments
Open

Minor enhancements #92

sajadhsm opened this issue Feb 27, 2022 · 2 comments

Comments

@sajadhsm
Copy link
Contributor

As I was reading through the source code, I've listed some minor issues which may improve the project:

  1. Route guards (Currently we can move to the login page by typing the address in the URL bar even when we are already logged in)
  2. mapValidationResponse does not handle 403 error
  3. No need to write <template #default> for suspense
  4. Enforce an import order convention
  5. Reset errors when re-submitting the forms
  6. useArticles composable:
    6.1. In fetchArticles function, since articleType can only have one value, it's better to use if-else rather than only if to skip other checks.
    6.2 getArticlesMeta function is acting like a composable. So maybe it's better to rename it to useArticlesMeta
  7. Vue docs doesn't recommend using generic argument for reactive(). So it may be better to change current generic usage.
  8. In AppLink component, useAttrs and v-bind="attrs" can be removed because of Fallthrough Attributes
  9. Add "Not Found" page

I would like to help fix these issues if they are valid to you. :D

@mutoe
Copy link
Owner

mutoe commented Feb 27, 2022

Your attentiveness is admirable!

Some of the content of this repository may seem less appropriate with Vue's update, and some of your suggestions are fine. My focus is not on Vue for the time being, if you can, PR welcome!

This was referenced Feb 27, 2022
@mutoe
Copy link
Owner

mutoe commented Jun 11, 2022

  1. In AppLink component, useAttrs and v-bind="attrs" can be removed because of Fallthrough Attributes

I think it is not a good idea to do this. This will lose the type of the component props. If IDE or VLS can deduce the type of the component's root element, I think it makes sense to do so, and until then, I don't think it's good practice.

I got it wrong :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants