nuxt / nuxt

The Intuitive Vue Framework.
https://nuxt.com
MIT License
54.97k stars 5.03k forks source link

TypeScript TypeDefs Suggestion for Nuxt 2.x #4050

Closed kevinmarrec closed 6 years ago

kevinmarrec commented 6 years ago

What problem does this feature solve?

Developing with both Nuxt & TypeScript, I lately have been likely frustrated using the Nuxt API around pages using TypeScript decorators, so here is what I want to suggest :

Provide Nuxt TypeDefs to be able to use (and get AutoCompletion on) the Nuxt Pages API (https://nuxtjs.org/api/ => "Pages") around TypeScript decorators for class-style Vue component (vue-class-component https://github.com/vuejs/vue-class-component ).

Example (Autocompletion on head API property through MetaInfo typedef of vue-meta):

Screenshot_5.png

nuxt-class-component would be not needed anymore, unless you want to put your Nuxt logic in your export default class without Editor Autocompletion and having trouble with TypeScript implict any !

What does the proposed changes look like?

For now I ended up with these definitions, according the official Nuxt API Documentation:

Screenshot_7.png

As you may have figured out, Context is declared as any type, it's just a question of time to define it properly according to Nuxt API Context (https://nuxtjs.org/api/context ).

I would need to know if what I did is relevant, I heard about types which were maybe present on 1.x but disappeared since 2.x release (#795), so I wasn't sure if my work might have already been done somewhere, maybe it was about another type of types ?

Then I'll be glad to finish my work and propose a PR.

This feature request is available on Nuxt community (#c7907)
kevinmarrec commented 6 years ago

Little update to let people what's going on here :

I will take time to build the type definition of the Nuxt Context, refering to https://nuxtjs.org/api/context.

Then I'll make a PR.

If you want to have Nuxt ComponentOptions TypeScript support right now in your project and you don't care (at least for now) having the Context declared as any type, you can simply add a nuxt.d.ts in a new @types folder in your Nuxt project :

// name-of-your-project/@types/nuxt.d.ts
import Vue from 'vue'
import { Route } from 'vue-router'
import { MetaInfo } from 'vue-meta'

declare module 'vue/types/options' {
  type Context = any
  interface Transition {
    name?: string
    mode?: string,
    css?: boolean,
    duration?: number,
    type?: string,
    enterClass?: string,
    enterToClass?: string,
    enterActiveClass?: string,
    leaveClass?: string,
    leaveToClass?: string,
    leaveActiveClass?: string
  }

  interface ComponentOptions<V extends Vue> {
    asyncData?: (ctx: Context) => object
    fetch?: (ctx: Context) => Promise<void> | void
    head?: MetaInfo | (() => MetaInfo)
    layout?: string | ((ctx: Context) => string)
    middleware?: string | string[]
    scrollToTop?: boolean
    transition?: string | Transition | ((to: Route, from: Route) => string)
    validate?: (ctx: Context) => Promise<boolean> | boolean
    watchQuery?: boolean | string[]
  }
}
NickBolles commented 6 years ago

@kevinmarrec what's the status of this? I would love this. If you have some progress, I would love to help out with fleshing out the types some more.

kevinmarrec commented 6 years ago

Hey @NickBolles , here is my updated definitions file :

import Vue from 'vue'
import { Route } from 'vue-router'
import { Dictionary } from 'vue-router/types/router';
import { Store } from 'vuex'
import { MetaInfo } from 'vue-meta'

declare module 'vue/types/options' {
  interface Context {
    app: Vue,
    isClient: boolean,
    isServer: boolean,
    isStatic: boolean,
    isDev: boolean,
    isHMR: boolean,
    route: Route,
    store: Store<any>,
    env: Object,
    params: Dictionary<string>,
    query: Dictionary<string>,
    req: Request,
    res: Response,
    redirect: Function,
    error: Function
    nuxtState: Object,
    beforeNuxtRender: Function
  }
  interface Transition {
    name?: string
    mode?: string,
    css?: boolean,
    duration?: number,
    type?: string,
    enterClass?: string,
    enterToClass?: string,
    enterActiveClass?: string,
    leaveClass?: string,
    leaveToClass?: string,
    leaveActiveClass?: string
  }

  interface ComponentOptions<V extends Vue> {
    asyncData?: (ctx: Context) => object
    fetch?: (ctx: Context) => Promise<void> | void
    head?: MetaInfo | (() => MetaInfo)
    layout?: string | ((ctx: Context) => string)
    middleware?: string | string[]
    scrollToTop?: boolean
    transition?: string | Transition | ((to: Route, from: Route) => string)
    validate?: (ctx: Context) => Promise<boolean> | boolean
    watchQuery?: boolean | string[]
  }
}

Feel free to take it and add modifications, my Context definition can be improved I think.

NickBolles commented 6 years ago

Perfect. Can we get a PR started and work through some improvements? I'll look at it later today, and I'd be willing to put it together.

kevinmarrec commented 6 years ago

Sure, I will make a PR in few hours, and will mention this issue πŸ‘

kevinmarrec commented 6 years ago

@NickBolles PR started

Mikkou commented 6 years ago

Thanks, @kevinmarrec

i have this error "decorator is not function" that is bind with this issue?

image

kevinmarrec commented 6 years ago

Hi @Mikkou , I don't think this is related to this issue, but it's still TypeScript related. Maybe you can find something helping you here ? https://github.com/rbuckton/reflect-metadata/issues/29 Old but maybe you have as mentionned in the last comment of the issue, created circular dependencies.

Feels free to open a new issue with a minimal example repository where the error occurs. Me or anyone would need more info about your configuration and files to know what's going on.

iwata commented 6 years ago

@kevinmarrec Here is my improvements.

import Vue from 'vue'
import {Store} from 'vuex'
import VueRouter, {Route} from 'vue-router'
import {MetaInfo} from 'vue-meta'

declare module 'vue/types/options' {
  interface ErrorParams {
    statusCode?: string
    message?: string
  }

  interface Context {
    app: Vue
    isClient: boolean
    isServer: boolean
    isStatic: boolean
    isDev: boolean
    isHMR: boolean
    route: Route
    store: Store<any>
    env: object
    query: Route['query']
    nuxtState: object
    req: Request
    res: Response
    params: Route['params']
    redirect(path: string, query?: Route['query']): void
    redirect(status: number, path: string, query?: Route['query']): void
    error(params: ErrorParams): void
    beforeNuxtRender({Conmponents, nuxtState}: any): void
  }

  interface Transition {
    name?: string
    mode?: string
    css?: boolean
    duration?: number
    type?: string
    enterClass?: string
    enterToClass?: string
    enterActiveClass?: string
    leaveClass?: string
    leaveToClass?: string
    leaveActiveClass?: string
  }

  interface ComponentOptions<V extends Vue> {
    layout?: string | ((ctx: Context) => string)
    middleware?: string | string[]
    scrollToTop?: boolean
    transition?: string | Transition | ((to: Route, from: Route) => string)
    head?: MetaInfo | (() => MetaInfo)
    fetch?(ctx: Context): Promise<void> | void
    asyncData?(ctx: Context): object
    validate?(ctx: Context): Promise<boolean> | boolean
  }
}
kevinmarrec commented 6 years ago

@iwata Thanks for sharing, did you see my last version ? https://github.com/nuxt/nuxt.js/pull/4164/files

Refering to my last code version, here are my thoughts about yours :

(πŸ‘Ž) env and nuxtState need to be dictionnaries, otherwise you will have error when typing env.something.

(πŸ‘Ž) My beforeNuxtRender seems to be better defined .(https://github.com/nuxt/nuxt.js/pull/4164/files#diff-82237d0cacfea01fd665277b59c45ce5R31)

(πŸ‘) Your redirect definitions made me see that my query? param is defined as object which is not the good type.

(πŸ‘) Refering other library typings, you caught a best practice I didn't follow in all my last code version, indeed interface properties which are functions need to be declare fn(param: any): void and not fn: (param: any) => void.

And some questions I want you to answer to know your point of view :

(❓) Is it recommended/best-practice to use things like Route['query'] to refer other library types ?

(❓) Regarding functions like redirect which have more than one definition (different signatures), is it best-practice to put the one with less parameters on the top ? (in my code I've something like you, but the two definitions are not ordered the same. https://github.com/nuxt/nuxt.js/pull/4164/files#diff-82237d0cacfea01fd665277b59c45ce5R27)

(❓) What is the best name between Error and ErrorParams (https://github.com/nuxt/nuxt.js/pull/4164/files#diff-82237d0cacfea01fd665277b59c45ce5R48)? Cause the interface looks like an error definition. I think both of them are good, but we need to take a decision πŸ˜„

(❓) I'm not 100% sure about if Error interface properties can be both optional, I'll need to check Nuxt source code.

=> When I'll get your feedback, I'll update my PR accordingly. And again, thank you for helping on this !

iwata commented 6 years ago

@kevinmarrec Thanks for your kind feedback.

did you see my last version ? https://github.com/nuxt/nuxt.js/pull/4164/files

Oh, sorry. I'll check it later.

[-] env and nuxtState need to be dictionnaries, otherwise you will have error when typing env.something.

Agree πŸ‘

[-] My beforeNuxtRender seems to be better defined

I checked your latest definition. Yeah, that's great ✨

[+] Your redirect definitions made me see that my query? param is defined as object which is not the good type. [+] Refering other library typings, you caught a best practice I didn't follow in all my last code version, indeed interface properties which are functions need to be declare fn(param: any): void and not fn: (param: any) => void.

πŸ‘

[?] Is it recommended/best-practice to use things like Route['query'] to refer other library types ?

Yes, you can check it in this article. typing - Is there a way to "extract" the type of TypeScript interface property? - Stack Overflow

[?] What is the best name between Error and ErrorParams

Well, I guess it's a trivial problem. I thought if it was Error it would be a bit too general. On the other hand, referring to the following sources, each properties of Error seems optional. https://github.com/nuxt/nuxt.js/blob/dev/packages/server/src/middleware/error.js#L9-L10

kevinmarrec commented 6 years ago

@iwata Perfect.

The only thing that still get me confused if that I'm not sure that the the source you mentionned is the good one to take in account (about ErrorParams properties).

Anyway, I'll test with making them optional, and try the different cases like only giving either the status or the message, and see how Nuxt handle it.

NickBolles commented 6 years ago

Looking good! I've been meaning to look at this but been busy on a big fix to another framework.

If your looking for Vue examples Vue Apollo has pretty good typings https://github.com/Akryum/vue-apollo/blob/97d317e765e2be3b751d35ba82d373cfd694c61a/types/vue.d.ts

kevinmarrec commented 6 years ago

@NickBolles πŸ‘ PR should be merge-ready this week, I think !

kevinmarrec commented 6 years ago

Feature implemented and merged through #4164

lock[bot] commented 5 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.