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

feat(reactivity): expose last result for computed getter #9497

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

johnsoncodehk
Copy link
Member

@johnsoncodehk johnsoncodehk commented Oct 28, 2023

computed internally uses Object.is to compare whether the old and new values ​​have changed. This is valid for primitives, but cannot compare objects (this is the correct behavior).

We can expose the computed last value for getter, so the user can decide whether to return the last value for object to pass Object.is.

const a = ref(1);
const b = ref(2);
const c = computed(oldValue => {
	if (oldValue && oldValue[0] === a.value && oldValue[1] === b.value) {
		return oldValue;
	}
	return [a.value, b.value];
});
import isEqual from 'lodash.isequal';

const a = ref(1);
const b = ref(2);
const c = computed(oldValue => {
	const newValue = { a: a.value, b: b.value };
	return isEqual(oldValue, newValue) ? oldValue : newValue;
});

Another alternative is to provide isEqual option, but I don't recommend this approach. This requires always fully executing the getter and returning a new object, which I think limits performance and GC pitfalls that the user should be able to avoid.

  • getter cannot exit early on equal to avoid possible expensive operations
  • getter may be executed frequently and return object to execute isEqual that needs to GC

@Anoesj
Copy link

Anoesj commented Oct 30, 2023

How much does redundant re-rendering affect performance compared to doing a (sometimes really heavy) isEquals-like check? Maybe it's wise to do some benchmarks to see if this API change truly improves performance in various scenarios. E.g.:

  • Small vs. huge objects/arrays/Map/Set/etcetera.
  • Small vs. big templates.

I don't think it's wise to provide an isEqual option, however, it may be wise to read some good docs on what kind of techniques truly improve performance and which ones don't. Also, the docs need to be really clear about only using this as an opt-in performance enhancement for non-primitives. Maybe some ESLint rules would also be nice, one for requiring usage of the oldValue parameter on every computed that returns a non-primitive, and one for warning about usage of oldValue on primitives.

@yyx990803
Copy link
Member

I'll keep this in mind when we update the docs for 3.4.

@yyx990803 yyx990803 merged commit 48b47a1 into vuejs:minor Oct 31, 2023
2 checks passed
@skirtles-code
Copy link
Contributor

I have some concerns about this addition. Given this PR has already been merged into minor, I've started a separate discussion at vuejs/rfcs#594, rather than posting it all here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants