dsuryd / dotNetify

Simple, lightweight, yet powerful way to build real-time web apps.
https://dotnetify.net
Other
1.17k stars 164 forks source link

Incorrect typings #255

Closed ColonelBundy closed 4 years ago

ColonelBundy commented 4 years ago

This line here https://github.com/dsuryd/dotNetify/blob/0cefadf2eca01a2355487f300645a58c5cfa822f/DevApp/src/dotnetify/_typings/index.ts#L2

React should not be imported here since in a vue project you'd never install react. Results in:

image

And also these should not be nullable: https://github.com/dsuryd/dotNetify/blob/0cefadf2eca01a2355487f300645a58c5cfa822f/DevApp/src/dotnetify/_typings/index.ts#L9-L11

Since this forces you to write ugly code like:

created() {
    this.vm = (dotnetify.vue as IDotnetifyVue).connect("HelloWorldVM", this, {
      watch: ["HelloWorld"],
    } as IConnectOptions);
  }
dsuryd commented 4 years ago

I couldn't find a solution that would let me remove the "react" import from the type definition without losing type support in React projects. If you can find one, send me a PR.

I can get rid of the nullable, but you can make your code better now by using the ! operator.

ColonelBundy commented 4 years ago

@dsuryd Since I'm not very familiar with react I don't feel confident fixing this without breaking the react version. I think a better way to do this would be to separate these into their own packages and use the core library as a dependency because as it stands right now it's completely broken for typescript.
This would also be better for maintainability since any contributions to the vue package wont impact the others.

Thoughts?

dsuryd commented 4 years ago

Is it completely broken for Vue projects? It isn't ideal, but won't it be a simple workaround to include "react" as devDependencies so it gets installed in node_modules just for the type def to work, but won't be included in the compiled app bundle?

dsuryd commented 4 years ago

Three steps to make a Vue project with Typescript work with dotNetify v4 type definitions:

  1. npm i -D @types/react.
  2. Add "skipLibCheck": true to tsconfig.json.
  3. Use ! operator on connect: dotnetify.vue!.connect(...) (won't be necessary in the next release)

I agree a separate package may be better, but it's not something I want to personally take on due to the increased maintenance burden. Anyone who's willing to maintain a fork is welcome.

ColonelBundy commented 4 years ago

@dsuryd Gotcha, I will probably put together another vue client to solve this properly in the future and maintain it.

Sidenote: thought about setting up a gitter chat?

dsuryd commented 4 years ago

Thanks. About gitter, never use it but I assume discussion isn't searchable (big negative) and live chatting will be personally disruptive.