Skip to content

Add a ...$updating property for each computed property. #29

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

Closed
wants to merge 1 commit into from
Closed

Add a ...$updating property for each computed property. #29

wants to merge 1 commit into from

Conversation

michaelzangl
Copy link
Contributor

This adds a property that is true while the real property is recomputing. Can be used as a visual loading indicator:

<div v-if="data$updating">
  <span class="fa fa-spin fa-spinner"></span> Loading data
</div>

@foxbenjaminfox
Copy link
Owner

foxbenjaminfox commented Mar 12, 2018

Sorry, it took me a while to get to this.

I like this PR, and I'd be happy to merge it. The one thing I'll quibble about is putting the $updating properties in the global instance namespace instead.

In my opinion it would be better to define a separate namespace for these properties, let's say perhaps this.$asyncComputedUpdating. (Or something shorter if you can think of a good name. Maybe just this.$updating.)

Then the new properties would be of the form this.$asyncComputedUpdating[key], instead of this[key + '$updating']. I think that makes things cleaner.

What do you think about this change?

Also, let's make sure that this feature is properly documented in the README. If you'd like to add a section about it to the README that would be great, or I could write something up if you'd rather.

@michaelzangl
Copy link
Contributor Author

Thanks for the reply

Putting everything in a separate namespace seems like a good idea, I'll look into it.

I can add a short example to the readme.

@gavinhungry
Copy link

@michaelzangl Any movement on this? I've been hacking together a loading indicator for asyncComputed properties, and this appears to be much cleaner.

If you haven't the time, I'd be happy to run with this and fulfill @foxbenjaminfox's proposal.

@michaelzangl
Copy link
Contributor Author

@gavinhungry
I am planning to solve this in the next two weeks for a new project.
My Idea is to add a $asyncComputed property to the vue instance and add a status object for each of the properties.
The status object should look like this:

{
 updating: true, /* while it is computing */
 error: true, /* If the promise was rejected during last evaluation */,
 success: true, /* if last evaluation was successful */
 exception: null, /* The exception that occured, while error is true and if available. null while error is false */
 recompute() { /* Not sure about this one, triggers refresh. We'll probably solve this cleaner using events */}
}

The idea is to be able to pass that object to a status component and that component will display the loading indicator or error message depending on the state.

@gavinhungry
Copy link

@michaelzangl - awesome, please @ me on that when you get to it!

@bramblex
Copy link

@gavinhungry

you do not need any hack, just wrap the async computed by wrapper.

example:
https://jsbin.com/bevofolope/edit?html,output

    function withIndicator(indicatorKey, asyncFunc) {
      return async function () {
        this[indicatorKey] = true
        try {
          return await asyncFunc.apply(this)
        } catch (err) {
          throw err
        } finally {
          this[indicatorKey] = false
        }
      }
    }

@michaelzangl
Copy link
Contributor Author

you do not need any hack, just wrap the async computed by wrapper.

This won't handle race conditions correctly

@michaelzangl
Copy link
Contributor Author

Closed in favor of #45

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.

4 participants