davestewart / vuex-pathify

Vue / Vuex plugin providing a unified path syntax to Vuex stores
https://davestewart.github.io/vuex-pathify
MIT License
1.37k stars 57 forks source link

Add Typescript typings file. Closes #7 #38

Closed ozum closed 5 years ago

ozum commented 5 years ago

Hi,

Added typescript typings for all parts of the library including

Also added minimal test for typings in types/test. To run:

$ npm run test:types

Module authr @davestewart did not include vue and vuex in dependencies, so I didn't. However, typings require vue and vuex installed, because of imports and I added them to devDependencies. @davestewart, if you see necessary, add vue and vuex to peer dependencies.

I tested typings in one of a vue project and it worked as expected. If I see points to be fixed, I will update it.

Any comments or reviews are welcome.

davestewart commented 5 years ago

Hey, that's amazing, thank you!

I will look at this this week.

It looks pretty intense. Was it difficult?

ozum commented 5 years ago

Your welcome. Not so difficult, but required some investigation and time.

davestewart commented 5 years ago

I wonder if some time I might convert the lib to TS.

I've got a few TS projects under my belt now so feel more comfortable with it.

BTW, have you seen this?

ozum commented 5 years ago

I wonder if some time I might convert the lib to TS.

IMHO, it is the way to go. Especially considering vue ecosystem is going towards TypeScript including vue itself will be written in TypeScript.

I've got a few TS projects under my belt now so feel more comfortable with it.

I always feel it is easier to code in TS than write definition files.

https://www.npmjs.com/package/vuex-module-decorators

Seems good. I also added decorator support to vuex-pathify in my project:

@Component
class ItemDetailsOptions extends Vue {
  ...
  @Get("name") name!: string; // Same as name: get('name') from `vuex-pathify`
  @Sync("age") age!: number;
  @Call("fn") fn!: () => string;
}

Creating a vue decorator requires vue-class-component. I can commit a PR after I test it within my project enough.

Considering developers not using vue-class-component, should I add it to another file like:

import { ... } from "vuex-pathify";
import { Get, Sync, Call } from "vuex-pathify/decorators";

Any thoughts?

Also I will thankful if you could solve #36 by adding accessor priority. I can update typings and decorators all in one go.

davestewart commented 5 years ago

if you see necessary, add vue and vuex to peer dependencies

I didn't know about peerDependencies - something I was wondering about, so perfect.

I also added decorator support to vuex-pathify in my project:

Decorators seem so damn cool, and obvious. I really must get into them! And thanks, that would be a very cool addition.

Considering developers not using vue-class-component, should I add it to another file like:

That makes a lot of sense!

Creating a vue decorator requires vue-class-component

So we tested class components at my last contract but found them to be rather limiting.

Maybe you can enlighten me!?

ozum commented 5 years ago

In last commit I added decorators and their documentation.

Considering developers not using vue-class-component, should I add it to another file

Rollup seems not to support multi bundles. So I developed another solution and add module requirement in try-catch block. So vue-class-component is not added to vuex-pathify. Decorators are imported from main package. If non-class component developers try to use decorators by mistake, decorator functions throw error to warn them.

import { Get, Sync, Call } from "vuex-pathify";

```js
@Component
class ItemDetailsOptions extends Vue {
  @Get("name") name!: string;
  @Sync("age") age!: number;
  @Call("fn") fn!: () => string;
}

Could you also verify this commit does not prevent compile of non-class-component based projects? It is very easy page opens or compile fails.

gil0mendes commented 5 years ago

An interesting change would be making the get function returning a typed GetAccessor, that would allow specifying the return type on the call:

export function get<T = any>(
  path: string | object,
  props?: string[] | object
): { get: GetAccessor<T> };

// (...)

type GetAccessor = <T = any>() => T;

The usage would look like the following, and with that type safety would be guaranteed:

computed: {
  isLoading: get<boolean>("categories/isLoading")
}
ozum commented 5 years ago

get function returning a typed GetAccessor

@gil0mendes, that's a good idea, thanks, I added it.

NarHakobyan commented 5 years ago

hi, any plans to merge this? :)

davestewart commented 5 years ago

Oh crap - really sorry - I've been slammed at work but my contract ends in 4 days.

I promise I'll look at this next week!

@ozum - is there anything I need to know about, or should this be a fairly easy merge?

What new things did you add?

Does anything new need to be documented?

ozum commented 5 years ago

is there anything I need to know about, or should this be a fairly easy merge?

It should be an easy merge.

Please review src/helpers/decorators.js. To make vue-class-component optional and adding decorator support at the same code base, I use a try/catch block. Test drive it with one of your previous project, if it compiles it is working as expected. If not, we should make vue-class-component mandatory, because it is requested for decorators in vue.

Other than that, all additions are TypeScript typings, which does not affect actual code.

What new things did you add?

import { Get, Sync, Call } from "vuex-pathify";

@Component
class ItemDetailsOptions extends Vue {
  @Get("name") name!: string;
  @Sync("age") age!: number;
  @Call("fn") fn!: () => string;
}

Does anything new need to be documented?

No, I added necessary docs for decorators.

davestewart commented 5 years ago

I'd like to convert the library to TypeScript at some point.

Hopefully it won't be too bothersome, but I'm sure I'll get tripped up at some point!

davestewart commented 5 years ago

BTW @ozum that decorator example looks sick (as in, amazing, if you don't know the slang)!

ozum commented 5 years ago

BTW @ozum that decorator example looks sick (as in, amazing, if you don't know the slang)!

Thanks.

I get some questions about why field names are repeated in decorators. Below is a more realistic example (similar from submitted docs).

import { Get, Sync, Call } from "vuex-pathify";

@Component
class ItemDetailsOptions extends Vue {
  @Get("products/name") name!: string;
  @Sync("person/age") age!: number;
  @Call("some/fn") fn!: () => string;
}
davestewart commented 5 years ago

What does the ! character signify?

ozum commented 5 years ago

It lets TS compiler assume that property is defined.

Otherwise TS complains that property is neither initialized nor assigned in class constructor.

ozum commented 5 years ago

Hi @davestewart. plan to merge?

davestewart commented 5 years ago

Merged. I've not checked as I'm so busy right now! Let me find some time tomorrow to publish.

Sorry for the long wait and thanks so much for your hard work!

davestewart commented 5 years ago

Just checked. It all looks great!

ozum commented 5 years ago

Your welcome.

davestewart commented 5 years ago

Released! https://twitter.com/dave_stewart/status/1105075672135933952

davestewart commented 5 years ago

One thing - are the docs wrong?

http://localhost:3000/#/api/property-decorator

The decorators should be caps, right?

ozum commented 5 years ago

It has been long time, and I checked source code and docs seems wrong in caps. You can change decorator names in docs to capital letter.