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

readonly(["a", "b", "c"]).includes() tracks dependencies even though array is nonreactive #2493 #6

Open
cuixiaorui opened this issue Jan 4, 2022 · 3 comments

Comments

@cuixiaorui
Copy link
Owner

cuixiaorui commented Jan 4, 2022

readonly(["a", "b", "c"]).includes() tracks dependencies even though array is nonreactive #2493

视频讲解

@likui628
Copy link

likui628 commented Jan 5, 2022

问题版本:

3.0.2

重现代码

codesandbox

import { readonly } from "vue";
import { effect } from "@vue/reactivity";

export default {
  setup() {
    const readonlyState = readonly(["a", "b", "c"]);

    effect(() => {
      //readonlyState[0] 不收集deps
      readonlyState.includes("a"); //收集deps
    });
  },
};

根据issue #2493,@lixiaofa的观点是readonly执行readonlyState[0]readonlyState.includes("a")行为应该一致,
都不应该进行依赖收集。

readonlyState.includes ("a") to collect dependencies, readonlystate [0] will not collect dependencies

问题分析

问题在baseHandlers.ts#L85-L88,target在执行getter时如果判断它时Array就会进一步检查key是否在arrayInstrumentations中。

const targetIsArray = isArray(target)

if (targetIsArray && hasOwn(arrayInstrumentations, key)) {
       //传入receiver改变了getter调用时的this值为target
	return Reflect.get(arrayInstrumentations, key, receiver)
}

baseHandlers.ts#L40-L71中,对数组执行includes会进行依赖收集。

const arrayInstrumentations: Record<string, Function> = {}
// instrument identity-sensitive Array methods to account for possible reactive
// values
;(['includes', 'indexOf', 'lastIndexOf'] as const).forEach(key => {
  const method = Array.prototype[key] as any
  arrayInstrumentations[key] = function(this: unknown[], ...args: unknown[]) {
    const arr = toRaw(this)
    for (let i = 0, l = this.length; i < l; i++) {
      track(arr, TrackOpTypes.GET, i + '') //track会将当前的effect加入对象deps列表中
    }
    // we run the method using the original args first (which may be reactive)
    const res = method.apply(arr, args)
    if (res === -1 || res === false) {
      // if that didn't work, run it again using raw values.
      return method.apply(arr, args.map(toRaw))
    } else {
      return res
    }
  }
})

所以为了解决这个问题,只要在判断条件中加上!isReadonly,这样对readonly的数组进行任何访问都不会进行依赖收集。
baseHandlers.ts#L103

const targetIsArray = isArray(target)

if (!isReadonly && targetIsArray && hasOwn(arrayInstrumentations, key)) {
	return Reflect.get(arrayInstrumentations, key, receiver)
}

@jp-liu
Copy link

jp-liu commented Jan 8, 2022

确保在只读数组中,普通数组方法不会触发依赖收集

翻译:

对应 issues: #2493

问题版本: 3.0.2

描述地址: fix(reactivity): ensure readonly on plain arrays doesn't track array methods

1.问题

import { readonly } from "vue";
import { effect } from "@vue/reactivity";

export default {
  setup() {
    const readonlyState = readonly(["a", "b", "c"]);

    effect(() => {
      //readonlyState[0] 不收集deps
      readonlyState.includes("a"); //收集deps
    });
  },
};

问题点:

  1. 读取 通过数组方法和下标获取,行为应该一致
  2. 只读 不应该收集依赖,因为不会改变,避免性能浪费

2.问题原因

2.1 readonly 只具备读取,不具备设置,所以不存在依赖变化

在我们使用 readonly 的时候,将值作为了 只读 , 之后我们只具备读取,而不具备设置的功能, 因为我们在 proxyset 方法实现的时候,对只读属性,根本就没有赋值操作, 这里的源码可以去看看 mini-vue/reactivity 的实现,可以看看自己的笔记

mini-vue之reactivity

2.2 只读数组,获取方法收集依赖,下标没收集依赖

只读数组通过下标和方法获取,产生了不一致的行为,方法获取的时候,进行了依赖收集,实际应该不需要

function createGetter(isReadonly = false, shallow = false) {
  return function get(target: Target, key: string | symbol, receiver: object) {
    // ...以上代码省略
    
    // 是否数组
    const targetIsArray = isArray(target)
    // 是否数组方法获取,是的话,返回重写的数组方法,在下面的代码中,太妙了
    // Reflect.get返回的是重写后的函数方法
    if (targetIsArray && hasOwn(arrayInstrumentations, key)) {
      return Reflect.get(arrayInstrumentations, key, receiver)
    }

    const res = Reflect.get(target, key, receiver)
    
    return res
  }
}
const arrayInstrumentations: Record<string, Function> = {}
;(['includes', 'indexOf', 'lastIndexOf'] as const).forEach(key => {
  // 保留原有方法
  const method = Array.prototype[key] as any
  arrayInstrumentations[key] = function(this: unknown[], ...args: unknown[]) {
    const arr = toRaw(this)
    for (let i = 0, l = this.length; i < l; i++) {
      // 数组元素,全量触发依赖收集
      track(arr, TrackOpTypes.GET, i + '')
    }
    // we run the method using the original args first (which may be reactive)
    // 执行原来方法
    const res = method.apply(arr, args)
    if (res === -1 || res === false) {
      // if that didn't work, run it again using raw values.
      return method.apply(arr, args.map(toRaw))
    } else {
      return res
    }
  }
})

2.3 分析

  1. 实际作为只读对象,我们不应该收集依赖,浪费内存空间,造成不必要的性能消耗, 只读对象没有 set 操作,所以也就不存在变化,触发更新的操作

  2. 这里只读数组,应该保持下标获取和方法操作的一致性,都是不进行依赖收集,除非是 shallowReadonly ,深层行为还是看具体使用

3. 解决方案

  1. 数组下标和方法获取,行为保持一致性,增加只读判定

    const targetIsArray = isArray(target)
    // 增加只读判定
    if (!isReadonly && targetIsArray && hasOwn(arrayInstrumentations, key)) {
    	return Reflect.get(arrayInstrumentations, key, receiver)
    }

    解决PR的代码baseHandlers.ts#L103

4. 总结

这个问题不算很难,知道它的原理,很容易定位到问题所在,进行处理,不过这里又学到一招,

@brenner8023
Copy link

brenner8023 commented Jan 9, 2022

在只读数组中,使用普通数组方法会触发依赖收集,而访问数组元素不会触发,行为不一致。

复现代码:
vue版本:3.0.2

import { readonly } from "vue";
import { effect } from "@vue/reactivity";
export default {
  name: "App",
  setup() {
    const arr = readonly(["a", "b", "c", "d"]);
    const eff = effect(() => {
      void arr[1];
      // arr.includes("c");
    });
    console.log(eff.deps.length);
  },
};

解决的方法:
增加只读标识的判断,只要数组是只读的,就不会走进去

const targetIsArray = isArray(target)
// if (targetIsArray && hasOwn(arrayInstrumentations, key)) {
if (!isReadonly && targetIsArray && hasOwn(arrayInstrumentations, key)) {
      return Reflect.get(arrayInstrumentations, key, receiver)
}

遇到的问题:
在vue3.2版本以后,eff的结构变了,需要通过eff.effect.deps.length,不再是eff.deps.length

@cuixiaorui cuixiaorui changed the title fix(reactivity): ensure readonly on plain arrays doesn't track array methods. (close: #2493) readonly(["a", "b", "c"]).includes() tracks dependencies even though array is nonreactive #2493 Jan 10, 2022
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

4 participants