Skip to content

Conversation

@pikax
Copy link
Member

@pikax pikax commented Sep 12, 2020

fix #517

Simplifying the logic of isReactive now every valid reactive v2 object will be a valid isReactive object.

The only issue with this is isReactive(obj) === true won't mean auto-unwrapping

const obj = Vue.observable({ a: ref(1) })
const reactiveObj = reactive({a: ref(1) })

isReactive(obj) // true

isReactive(reactiveObj) // true


obj.a // {value:1}
reactiveObj.a // 1

@pikax pikax requested a review from antfu September 15, 2020 13:36
antfu
antfu previously approved these changes Sep 15, 2020
Comment on lines 18 to 24
return (
(obj &&
typeof obj === 'object' &&
'__ob__' in obj &&
!obj.__ob__.__raw__) ||
false
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (
(obj &&
typeof obj === 'object' &&
'__ob__' in obj &&
!obj.__ob__.__raw__) ||
false
)
return Boolean(obj?.__ob__ && !obj.__bo__?.__raw__)


export function isRaw(obj: any): boolean {
return rawSet.has(obj)
return obj && typeof obj === 'object' && '__ob__' in obj && obj.__ob__.__raw__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return obj && typeof obj === 'object' && '__ob__' in obj && obj.__ob__.__raw__
return Boolean(obj?.__ob__?.__raw__)

const ob = (observed as any).__ob__

for (const key of Object.keys(obj)) {
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? I think we can make the function arg to (obj: any): T and get rid of those as any then.


return (observed as any).__ob__.value || observed
return (
((observed as any).__ob__ && (observed as any).__ob__.value) || observed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
((observed as any).__ob__ && (observed as any).__ob__.value) || observed
(observed as any)?.__ob__?.value) || observed

src/mixin.ts Outdated
// mark props
markReactive(props)
// fake reactive for `toRefs(props)`
def(props, '__props_reactive__', true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put __props_reactive__ inside src/utils/symbols.ts instead of a magic string 😄

@antfu antfu self-requested a review September 15, 2020 15:08
@antfu antfu dismissed their stale review September 15, 2020 15:09

Sorry wrong button 😅

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.

App breaks after upgrade from 1.0.0-beta.11 to 1.0.0-beta.12 and later

2 participants