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

The type definition of $loading conflicts with Buefy's one #49

Closed
iwata opened this issue Jan 29, 2019 · 40 comments
Closed

The type definition of $loading conflicts with Buefy's one #49

iwata opened this issue Jan 29, 2019 · 40 comments
Labels
transfered Issue transfered from nuxt main repo types The issue or pull request is related to `types` package wontfix This will not be worked on

Comments

@iwata
Copy link

iwata commented Jan 29, 2019

Version

v2.4.0

Reproduction link

https://github.com/iwata/codesandbox-nuxt

Steps to reproduce

$ git clone git@github.com:iwata/codesandbox-nuxt.git
$ yarn install
$ yarn dev

What is expected ?

No type checking errors

What is actually happening?

It shows this error:

ERROR in /Users/iwata/.go/src/github.com/iwata/codesandbox-nuxt/node_modules/@nuxt/vue-app/types/index.d.ts(73,18):
TS2430: Interface 'NuxtApp' incorrectly extends interface 'Vue'.
  Types of property '$loading' are incompatible.
    Property 'open' is missing in type 'NuxtLoading' but required in type '{ open: (params: LoadingConfig) => { close: () => any; }; }'.

Additional comments?

If you import Buefy looks like plugins/buefy.ts, it declares $loading for Vue instance.
https://github.com/buefy/buefy/blob/dev/types/index.d.ts#L10
So NuxtApp extends the Vue, therefore $loading conflicts.
https://github.com/nuxt/nuxt.js/blob/v2.4.0/packages/vue-app/types/index.d.ts#L73-L74

This bug report is available on Nuxt community (#c8559)
@kevinmarrec
Copy link
Contributor

kevinmarrec commented Jan 29, 2019

Well, there is the same issue for ElementUI if i'm right.

I don't know what we're supposed to do, both of frameworks (Nuxt & Buefy) are setuping the same variable so 😬

@manniL WDYT ?

The only solution could be to have the $loading definition outside of @nuxt/vue-app to avoid that but it sounds weird to not have it inside.

@manniL
Copy link
Member

manniL commented Jan 29, 2019

@kevinmarrec The problem lies even deeper. If ElementUI and Buefy both use $loading, will they override the Nuxt.js $loading. Or will Nuxt override theirs?

We should think about how to properly avoid naming collisions like that.

@iwata
Copy link
Author

iwata commented Jan 29, 2019

Is it necessary to have a plugin namespace in Vue first?

@kevinmarrec
Copy link
Contributor

@manniL I don't know, I guess they override Nuxt as they are Vue plugins (Vue.use) and instanciated on top of Nuxt ?

@sebkolind
Copy link

I have this issue with Element-ui also. Are there any clarifications on this? Maybe a temporary fix?

@kevinmarrec
Copy link
Contributor

kevinmarrec commented Feb 5, 2019

Sorry if we didn't find a workaround yet.

UI libraries shouldn't directly non-namespaced add properties to the Vue instance interface.
For example, Vuetify uses this.$vuetify.x, meanwhile Buefy & ElementUI directly use this.x, which is IMO not good. (this being a Vue instance)

The issue is that these 2 UI libraries can't change this behavior without breaking changes.

@sebkolind
Copy link

@kevinmarrec I understand, and I agree with you. Is there a way for us to make a workaround?

@manniL
Copy link
Member

manniL commented Feb 5, 2019

@SebastianKS Ideally creating issues in the corresponding repos 🙈

@hartmut-co-uk
Copy link

hartmut-co-uk commented Feb 5, 2019

I run into the issue with Element-ui as well.
I guess namespacing sounds like the way to go - but as @kevinmarrec mentioned this will come with breaking changes - so if rolled out as @deprecated via (several / major) releases until removed will take time..
Maybe it would be a good idea to escalate this to the vuejs team directly - to add documentation / guidelines to avoid such clashes going forward and with more frameworks, .. to come.

@bichikim
Copy link

bichikim commented Feb 12, 2019

I believe this is not a Nuxt-ts or element-ui issue
@iwata @hartmut-co-uk may you need to define the $loading type

import {ElLoading} from 'element-ui/types/loading'
import {ElMessage} from 'element-ui/types/message'
import {ElMessageBox} from 'element-ui/types/message-box'
import {ElNotification} from 'element-ui/types/notification'
declare module 'vue/types/vue' {
  interface Vue {
     readonly $loading: typeof ElLoading.service
     // maybe you will use these things also
     readonly $alert: typeof ElMessageBox.alert
     readonly $confirm: typeof ElMessageBox.confirm
     readonly $message: ElMessage
     readonly $msgbox: ElMessageBox
     readonly $notify: ElNotification
     readonly $prompt: typeof ElMessageBox.prompt
  }
}

@manniL
Copy link
Member

manniL commented Feb 12, 2019

@bichikim The issue is that it's a naming collision between the $loading from nuxt and the one from ElementUI/Buefy.

@bichikim
Copy link

@manniL Oh i see~ in that case, fix element-ui loading at element-ui/lib/loading

@arambert
Copy link

This error prevents the command nuxt-ts generate from working...

In my case, it's conflicting with Buefy.
So as a dirty temporary fix, I've edited node_modules/@nuxt/vue-app-edge/types/index.d.ts with:

+export interface LoadingConfig {
+}

export interface NuxtLoading extends Vue {
  fail?(): void;
  finish(): void;
  increase?(num: number): void;
  pause?(): void;
  start(): void;
+  open: (params: LoadingConfig) => { close: () => any; };
}

@P-de-Jong
Copy link

For now i added the following to my tsconfig.json:

"compilerOptions": {
    "skipLibCheck": true
}

@AshleyCao
Copy link

@P-de-Jong's solution works for me

@nicbavetta
Copy link

@P-de-Jong's solution works for me

This also worked for me, however, it seems to be more of a work-around than a solution.

@manniL
Copy link
Member

manniL commented Mar 24, 2019

We will change it to $nuxt.$loading this.$nuxt.$progress in Nuxt 3 ☺️

@kevinmarrec
Copy link
Contributor

kevinmarrec commented Mar 25, 2019

@manniL In fact it's already this.$nuxt.$loading, and this.$loading is only a shortcut (AFAIK).
$nuxt being a Vue instance, in any case we need to say that NuxtApp ($nuxt) extends Vue.

The issue is IMO 100% Buefy fault which is not extending but overriding Vue typedefs through https://github.com/buefy/buefy/blob/dev/types/index.d.ts#L10.

By doing this, they force the Vue instance to always have $loading property and also forcing it to be of type LoadingProgrammatic, which is wrong.
Meanwhile within Nuxt, we only say that $nuxt is a Vue instance with a $loading property.

Unless we force us to change the name from $loading to something else, there won't be workarounds.
Buefy (and ElementUI) should highly initiate a breaking change which namespace their properties.

@manniL
Copy link
Member

manniL commented Mar 25, 2019

@kevinmarrec Whoops. Updated my statement based on the things discused in the last meeting☺️

@noe132
Copy link

noe132 commented Mar 26, 2019

since if $loading is only used in NuxtApp
this could be a temp workaround
but won't be a perma fix if there is any naming collision in the future

interface NuxtAppAnyLoading extends Vue {
  $loading: any
}

export interface NuxtApp extends NuxtAppAnyLoading {
  $loading: NuxtLoading
  isOffline: boolean
  isOnline: boolean
}

@stale stale bot closed this as completed Apr 27, 2019
@manniL manniL reopened this Apr 29, 2019
@nuxt nuxt deleted a comment from stale bot May 1, 2019
@nuxt nuxt deleted a comment from stale bot May 1, 2019
@iwata
Copy link
Author

iwata commented Aug 2, 2019

I confirmed fixed issuse with Buefy v0.8.0.

@iwata iwata closed this as completed Aug 2, 2019
@georgyfarniev
Copy link

This issue still persists with element-ui

@coolsam726
Copy link

This issue still persists with element-ui

For Element-UI, the only work-around that got me out of the mess is to override Nuxt's $loading. I did this by creating a new type file in my app and defining an interface which extends NuxtApp type, where I defined the type of $loading as 'any':

In file ~/types/nuxt.d.ts

import { NuxtApp } from "@nuxt/vue-app/types/index";

export interface NuxtLoading extends NuxtApp {
  $loading: any
}

@pi0 pi0 transferred this issue from nuxt/nuxt Aug 10, 2019
@ChangJoo-Park
Copy link

ChangJoo-Park commented Aug 22, 2019

This issue still persists with element-ui

For Element-UI, the only work-around that got me out of the mess is to override Nuxt's $loading. I did this by creating a new type file in my app and defining an interface which extends NuxtApp type, where I defined the type of $loading as 'any':

In file ~/types/nuxt.d.ts

import { NuxtApp } from "@nuxt/vue-app/types/index";

export interface NuxtLoading extends NuxtApp {
  $loading: any
}

i tried import { NuxtApp } from "@nuxt/vue-app/types/index";
but can't find @nuxt/vue-app 😭

import { NuxtApp } from "@nuxt/types/app";

export interface NuxtLoading extends NuxtApp {
  $loading: any
}

this is no error, still not working

91:18 Interface 'NuxtApp' incorrectly extends interface 'Vue'.
  Types of property '$loading' are incompatible.
    Type 'NuxtLoading' is not assignable to type '(options: LoadingServiceOptions) => ElLoadingComponent'.
      Type 'NuxtLoading' provides no match for the signature '(options: LoadingServiceOptions): ElLoadingComponent'.
    89 | }
    90 |
  > 91 | export interface NuxtApp extends Vue {
       |                  ^
    92 |   $options: NuxtAppOptions
    93 |   $loading: NuxtLoading
    94 |   isOffline: boolean

@kevinmarrec
Copy link
Contributor

@ChangJoo-Park

Since new TS support, NuxtApp is now accessible via

import { NuxtApp } from '@nuxt/types/app/index'

@ChangJoo-Park
Copy link

Thanks @kevinmarrec

@dantio
Copy link

dantio commented Sep 11, 2019

@kevinmarrec Doesn't solve my problem :/

In file ~/types/nuxt.d.ts

import { NuxtApp } from '@nuxt/types/app/index'

export interface NuxtLoading extends NuxtApp {
  $loading: any
}

nuxt: 2.9.2
element-ui: 2.4.11

Do I need to add nuxt.d.ts somewhere in tsconfig.json?

@dmix
Copy link

dmix commented Sep 13, 2019

@danito as a temporary bypass I just forked the @nuxt/types and edited:

node_modules/@nuxt/types/app/index.d.ts:91

From:

  $loading: NuxtLoading

To:

  $loading: any

@kevinmarrec kevinmarrec reopened this Sep 13, 2019
@dmix
Copy link

dmix commented Sep 13, 2019

Has there been an issue open on ElementUI's repos about this? I'd imagine this is something they could fix although Nuxt locking down a generic global name like $loading might cause future issues. Although it's difficult to say where that line should be or not.

@kevinmarrec kevinmarrec added types The issue or pull request is related to `types` package wontfix This will not be worked on labels Sep 16, 2019
@dmix
Copy link

dmix commented Oct 9, 2019

For the record the new issue on ElementUI is located here: ElemeFE/element#17630

@JohannesLichtenberger
Copy link

BTW: Is Element-UI actively maintained or should I switch to another UI-framework? No commit since 23 days seems to be strange for such a big project!?

@kevinmarrec
Copy link
Contributor

23 days seems indeed strange.

@kevinmarrec
Copy link
Contributor

Has anyone found an official workaround (that may already present in this thread) that currently works ?

@JohannesLichtenberger
Copy link

In tsconfig.json:

"compilerOptions": {
    "skipLibCheck": true
}

works for me, but hmm

Maybe instead,

"paths": {
    "element-ui": [".sink.d.ts"] // empty file
}

works, too?

@dmix
Copy link

dmix commented Oct 24, 2019

The real solution is either Nuxt or ElementUI needs to fix their namespace clashing and pick a less generic name than "loading" for root-level global stuff.

But yes, either edit the ts configs as above or fork Nuxt and change NuxtApp's interface to $loading: any

@skyrpex
Copy link

skyrpex commented Oct 31, 2019

Maybe it would be worth to start thinking in terms of the composition API? If Nuxt and/or ElementUI provided this stuff through a composable there would be no problem.

For example, I'm relying on a custom made const nuxt = useNuxtContext() composable. I access everything related to Nuxt using that.

@dmix
Copy link

dmix commented Nov 9, 2019

That would be ideal although legacy support for the current Vue 2 is a long term goal for the team I believe.

@skyrpex
Copy link

skyrpex commented Nov 10, 2019

Yes, that's still a problem.

@kevinmarrec
Copy link
Contributor

kevinmarrec commented Dec 8, 2019

As it's a unfixable issue around the module itself, i'm closing it here. Please open an issue around Nuxt core repo (there may already be one) about changing loading Nuxt API for next Nuxt version, if it seems the most relevant thing to do.

@zhangxuchuan827
Copy link

oh . it is so sad。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
transfered Issue transfered from nuxt main repo types The issue or pull request is related to `types` package wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests