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

Order of operations matters when using computed properties and a formatter #658

Closed
davisg123 opened this issue Aug 8, 2016 · 14 comments
Closed

Comments

@davisg123
Copy link

Specifying the computed property dependency before a number formatter and the | call results in the property not being recomputed

https://jsfiddle.net/64ta6qzz/18/

@jccazeaux
Copy link
Contributor

The idea of call formatter is to send arguments to function. The function will work with the arguements instead of getting the data in the model. With arguments Rivets will be able to bind all necessary data. You won't have to use the computed properties notation.
In your example this will be
https://jsfiddle.net/jccazeaux/j1dqe2d9/1/

@jccazeaux
Copy link
Contributor

In fact, the only usage of computed properties is
state.remainingTime < state.currentTime | call | formatter
In the following expression
state.remainingTime | call | formatter < state.currentTime
< and state.currentTime are handled as parameters for the formatter, not as computed properties. So it works because when state.currentTime changes, rivets will reexecute the expression.
In the correct notation in fact it works, but since 0.9 functions are not executed so when reevaluating the state.remaingTime it will return the same function as a pointer. The dataBinding won't be executed because the value (pointer on function) has not changed. See #572 it's the same problem here.
The good news is, when PR mikeric/sightglass#16 is merged, the bug will be fixed !

@benadamstyles
Copy link
Collaborator

@jccazeaux mikeric/sightglass#16 is merged, but I'm not sure how to release.

@benadamstyles
Copy link
Collaborator

Should we change the docs on computed properties to make it clear that it doesn't need to be used if you are using | call?

@jccazeaux
Copy link
Contributor

To release sightglass it must be pushed in npm as a 0.2.6. Then in rivets' package.json just reference this 0.2.6 before the build.
Yes the doc must be updated, i'll handle it with 0.9.2 update

@benadamstyles
Copy link
Collaborator

It is also on bower and component – shall we just ignore them?

@jccazeaux
Copy link
Contributor

We should keep the version up to date in all the repo. Is sightglass really used outside rivets?

@benadamstyles
Copy link
Collaborator

I use it outside rivets 😛

@jccazeaux
Copy link
Contributor

@Leeds-eBooks has sightglass been published?

@benadamstyles
Copy link
Collaborator

No, will do it asap

@jccazeaux
Copy link
Contributor

OK, package.json must be updated to use it for the rivets.bundle.min.js?

@benadamstyles
Copy link
Collaborator

Yeah I guess I'll have to publish 0.9.4 after I've released sightglass?

@benadamstyles
Copy link
Collaborator

Ah, totally didn't know that bower fetches the latest version from github automatically!

@benadamstyles
Copy link
Collaborator

Ok updated sightglass is bundled in 0.9.4

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

No branches or pull requests

3 participants