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

Add a decorator for vue-class-component #88

Merged
merged 1 commit into from
Dec 27, 2019

Conversation

nwtgck
Copy link
Contributor

@nwtgck nwtgck commented Dec 26, 2019

Made a decorator for vue-class-component

The implementation is based on #25 (comment). Thank you very much @saraedum and @TheNoim!

Here is a usage.

import { AsyncComputedProp } from "vue-async-computed";

...

  @AsyncComputedProp()
  private async myText(): Promise<string> {
    ...
  }
...

I'd like to use @AsyncComputed() instead of @AsyncComputedProp(), but AsyncComputed is already defined in the scope. Does someone have a better name than @AsyncComputedProp ?

@foxbenjaminfox foxbenjaminfox merged commit 0e76b7b into foxbenjaminfox:master Dec 27, 2019
@foxbenjaminfox
Copy link
Owner

Thanks! I've merged the PR, although I intend to make a few modifications before making a release with it.

Specifically, I'm not too keen on adding a dependency on vue-class-component. I kind of like the fact that vue-async-computed is entirely free of runtime dependencies, and in any case it seems not so good to force even people who don't use the decorator to depend on vue-class-component. But I see that all we need from it is the createDecorator function, whose definition seems pretty simple, and can easily be inlined.

It is true that this involves depending on the internals of the vue-class-component implementation, which normally I'd avoid doing. But that's a bullet I've already bitten with vue-async-computed: this is a library which depends very heavily on the internals of Vue already. It is true that this is a maintenance burden, but it is a maintenance burden that already applies, and extending it a little bit further seems acceptable to me.

I think I'll also shuffle some things around so that I can export the decorator as AsyncComputed instead of AsyncComputedProp. That doesn't seem so hard.

@foxbenjaminfox
Copy link
Owner

My previous comment was a bit over-optimistic I think. I've decided that it's probably best that the decorator lives in a separate npm package, which provides the separation I want here, while still having an "official" implementation of this decorator.

As such, I've released v3.8.3 of vue-async-computed which exports more of the types (including IAsyncComputedValue<T>), and I've moved your decorator implementation into its own package, which I'm about to release.

foxbenjaminfox added a commit to foxbenjaminfox/vue-async-computed-decorator that referenced this pull request Dec 27, 2019
This takes the decorator implementation contributed in
foxbenjaminfox/vue-async-computed#88 and
puts it into its own package. It also adds a few tests for the decorator.
@foxbenjaminfox
Copy link
Owner

I've released a v0.0.1 of vue-async-computed-decorator, which is the package into which I split out this decorator. (I've also added there some tests and documentation.)

Let me know if there is any trouble using it, and thanks again for the contribution!

@nwtgck
Copy link
Contributor Author

nwtgck commented Dec 27, 2019

@foxbenjaminfox Very greate decision I think! Honestly, I'd like to split the package too or have multiple packages in one repository. I'll use vue-async-computed-decorator@0.0.1.

@nwtgck nwtgck deleted the decorator branch December 27, 2019 13:56
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

Successfully merging this pull request may close these issues.

None yet

2 participants