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

fix(reactive): use isExtensible instead of isFrozen #1753

Merged
merged 3 commits into from
Aug 5, 2020

Conversation

wujieZ
Copy link
Contributor

@wujieZ wujieZ commented Jul 31, 2020

use isExtensible instead of isFrozen

@wujieZ wujieZ changed the title fix: use isExtensible instead of isFrozen fix(reactive): use isExtensible instead of isFrozen Aug 3, 2020
@yyx990803
Copy link
Member

A non-extensible object may still be mutable, and can be observed.

Also Vue 2 uses the same Object.isFrozen check and Vue 3 should keep it consistent.

@yyx990803 yyx990803 closed this Aug 3, 2020
@wujieZ
Copy link
Contributor Author

wujieZ commented Aug 4, 2020

thanks for commented! ! but....

  1. when I try to reactive a non-extensible object
const { reactive } = Vue
const obj = reactive(Object.preventExtensions({a: 1}))

it will cause an error, beacuse a non-extensible object may not be a Frozen obejct, so we try to define a property(__v_reactive) for a non-extensible object in source code.

- Uncaught TypeError: Cannot define property __v_reactive, object is not extensible
    at Function.defineProperty (<anonymous>)
    at def (vue.global.js:327)
    at createReactiveObject (vue.global.js:970)
    at reactive (vue.global.js:927)
    at setup (test.html:13)
    at callWithErrorHandling (vue.global.js:1277)
    at setupStatefulComponent (vue.global.js:6814)
    at setupComponent (vue.global.js:6775)
    at mountComponent (vue.global.js:5107)
    at processComponent (vue.global.js:5083)
  1. In Vue2, we also use Object.isExtensible to check。
export function observe (value: any, asRootData: ?boolean): Observer | void {
  if (!isObject(value) || value instanceof VNode) {
    return
  }
  let ob: Observer | void
  if (hasOwn(value, '__ob__') && value.__ob__ instanceof Observer) {
    ob = value.__ob__
  } else if (
    shouldObserve &&
    !isServerRendering() &&
    (Array.isArray(value) || isPlainObject(value)) &&
    Object.isExtensible(value) &&     //   use Object.isExtensible to check
    !value._isVue
  ) {
    ob = new Observer(value)
  }
  if (asRootData && ob) {
    ob.vmCount++
  }
  return ob
}

@yyx990803 yyx990803 merged commit 2787c34 into vuejs:master Aug 5, 2020
@wujieZ wujieZ deleted the reactive-canObserve branch August 6, 2020 10:47
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.

2 participants