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

Make pureComputed the default #23

Open
brianmhunt opened this issue May 17, 2017 · 0 comments
Open

Make pureComputed the default #23

brianmhunt opened this issue May 17, 2017 · 0 comments

Comments

@brianmhunt
Copy link
Member

From https://github.com/knockout/tko.computed/issues/1

From knockout/knockout#2167 by @futpib

I'm moving this here since this is where it could be accepted (for ko-4).

Make pureComputed the default computed and make computed impureComputed or even `autorun

I find that, when choosing between pureComputed and computed, pureComputed is almost always the right one due to laziness and performance benefits.

You really want computed only when you want immediate and predictable evaluation for side-effects.

Also you almost certainly want to break your computed in two parts with ignoreDependencies to make sure evaluation happens only when you want:

ko.computed(() => {
    // register dependencies
    ko.ignoreDependencies(() => {
        // produce side-effects
    });
});

This is why I propose that computed is renamed to a longer (and scarier) impureComputed or autorun (like in mobx), possibly changing it's signature dividing "register dependencies" and "produce side-effects" parts. And pureComputed is renamed to computed to make it the go-to option for less experienced knockout.js developers. Of course this would break compatibility, so this is probably only feasible for the next major release.

If we could make documentation advise towards pureComputed and against computed when in doubt, that would be great.

If it were possible to ban any non-local mutation in pureComputed's read and write functions (except writing other observables in write), I'd also root for that.

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

1 participant