Skip to content

refactor(reactivity, runtime-core): improve performance-critical code #13274

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

Open
wants to merge 1 commit into
base: vapor
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ import {
} from '../src'
import type { ComputedRef, ComputedRefImpl } from '../src/computed'
import { pauseTracking, resetTracking } from '../src/effect'
import { SubscriberFlags } from '../src/system'
import { ReactiveFlags } from '../src/system'

describe('reactivity/computed', () => {
it('should return updated value', () => {
@@ -467,12 +467,8 @@ describe('reactivity/computed', () => {
const c2 = computed(() => c1.value) as unknown as ComputedRefImpl

c2.value
expect(
c1.flags & (SubscriberFlags.Dirty | SubscriberFlags.PendingComputed),
).toBe(0)
expect(
c2.flags & (SubscriberFlags.Dirty | SubscriberFlags.PendingComputed),
).toBe(0)
expect(c1.flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)).toBe(0)
expect(c2.flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)).toBe(0)
})

it('should chained computeds dirtyLevel update with first computed effect', () => {
4 changes: 2 additions & 2 deletions packages/reactivity/__tests__/effect.spec.ts
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ import {
stop,
toRaw,
} from '../src/index'
import { type Dependency, endBatch, startBatch } from '../src/system'
import { type ReactiveNode, endBatch, startBatch } from '../src/system'

describe('reactivity/effect', () => {
it('should run the passed function once (wrapped by a effect)', () => {
@@ -1178,7 +1178,7 @@ describe('reactivity/effect', () => {
})

describe('dep unsubscribe', () => {
function getSubCount(dep: Dependency | undefined) {
function getSubCount(dep: ReactiveNode | undefined) {
let count = 0
let sub = dep!.subs
while (sub) {
66 changes: 32 additions & 34 deletions packages/reactivity/__tests__/effectScope.spec.ts
Original file line number Diff line number Diff line change
@@ -2,13 +2,15 @@ import { nextTick, watch, watchEffect } from '@vue/runtime-core'
import {
type ComputedRef,
EffectScope,
ReactiveEffect,
computed,
effect,
effectScope,
getCurrentScope,
onScopeDispose,
reactive,
ref,
setCurrentScope,
} from '../src'

describe('reactivity/effect/scope', () => {
@@ -20,7 +22,7 @@ describe('reactivity/effect/scope', () => {

it('should accept zero argument', () => {
const scope = effectScope()
expect(scope.effects.length).toBe(0)
expect(getEffectsCount(scope)).toBe(0)
})

it('should return run value', () => {
@@ -29,7 +31,8 @@ describe('reactivity/effect/scope', () => {

it('should work w/ active property', () => {
const scope = effectScope()
scope.run(() => 1)
const src = computed(() => 1)
scope.run(() => src.value)
expect(scope.active).toBe(true)
scope.stop()
expect(scope.active).toBe(false)
@@ -47,7 +50,7 @@ describe('reactivity/effect/scope', () => {
expect(dummy).toBe(7)
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)
})

it('stop', () => {
@@ -60,7 +63,7 @@ describe('reactivity/effect/scope', () => {
effect(() => (doubled = counter.num * 2))
})

expect(scope.effects.length).toBe(2)
expect(getEffectsCount(scope)).toBe(2)

expect(dummy).toBe(0)
counter.num = 7
@@ -87,9 +90,8 @@ describe('reactivity/effect/scope', () => {
})
})

expect(scope.effects.length).toBe(1)
expect(scope.scopes!.length).toBe(1)
expect(scope.scopes![0]).toBeInstanceOf(EffectScope)
expect(getEffectsCount(scope)).toBe(1)
expect(scope.deps?.nextDep?.dep).toBeInstanceOf(EffectScope)

expect(dummy).toBe(0)
counter.num = 7
@@ -117,7 +119,7 @@ describe('reactivity/effect/scope', () => {
})
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

expect(dummy).toBe(0)
counter.num = 7
@@ -142,13 +144,13 @@ describe('reactivity/effect/scope', () => {
effect(() => (dummy = counter.num))
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

scope.run(() => {
effect(() => (doubled = counter.num * 2))
})

expect(scope.effects.length).toBe(2)
expect(getEffectsCount(scope)).toBe(2)

counter.num = 7
expect(dummy).toBe(7)
@@ -166,21 +168,21 @@ describe('reactivity/effect/scope', () => {
effect(() => (dummy = counter.num))
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

scope.stop()

expect(getEffectsCount(scope)).toBe(0)

scope.run(() => {
effect(() => (doubled = counter.num * 2))
})

expect('[Vue warn] cannot run an inactive effect scope.').toHaveBeenWarned()

expect(scope.effects.length).toBe(0)
expect(getEffectsCount(scope)).toBe(1)

counter.num = 7
expect(dummy).toBe(0)
expect(doubled).toBe(undefined)
expect(doubled).toBe(14)
})

it('should fire onScopeDispose hook', () => {
@@ -224,9 +226,9 @@ describe('reactivity/effect/scope', () => {
it('should dereference child scope from parent scope after stopping child scope (no memleaks)', () => {
const parent = effectScope()
const child = parent.run(() => effectScope())!
expect(parent.scopes!.includes(child)).toBe(true)
expect(parent.deps?.dep).toBe(child)
child.stop()
expect(parent.scopes!.includes(child)).toBe(false)
expect(parent.deps).toBeUndefined()
})

it('test with higher level APIs', async () => {
@@ -290,21 +292,7 @@ describe('reactivity/effect/scope', () => {

parentScope.run(() => {
const childScope = effectScope(true)
childScope.on()
childScope.off()
expect(getCurrentScope()).toBe(parentScope)
})
})

it('calling on() and off() multiple times inside an active scope should not break currentScope', () => {
const parentScope = effectScope()
parentScope.run(() => {
const childScope = effectScope(true)
childScope.on()
childScope.on()
childScope.off()
childScope.off()
childScope.off()
setCurrentScope(setCurrentScope(childScope))
expect(getCurrentScope()).toBe(parentScope)
})
})
@@ -372,7 +360,17 @@ describe('reactivity/effect/scope', () => {
expect(watcherCalls).toBe(3)
expect(cleanupCalls).toBe(1)

expect(scope.effects.length).toBe(0)
expect(scope.cleanups.length).toBe(0)
expect(getEffectsCount(scope)).toBe(0)
expect(scope.cleanupsLength).toBe(0)
})
})

function getEffectsCount(scope: EffectScope): number {
let n = 0
for (let dep = scope.deps; dep !== undefined; dep = dep.nextDep) {
if (dep.dep instanceof ReactiveEffect) {
n++
}
}
return n
}
76 changes: 0 additions & 76 deletions packages/reactivity/__tests__/watch.spec.ts
Original file line number Diff line number Diff line change
@@ -3,40 +3,12 @@ import {
type Ref,
WatchErrorCodes,
type WatchOptions,
type WatchScheduler,
computed,
onWatcherCleanup,
ref,
watch,
} from '../src'

const queue: (() => void)[] = []

// a simple scheduler for testing purposes
let isFlushPending = false
const resolvedPromise = /*@__PURE__*/ Promise.resolve() as Promise<any>
const nextTick = (fn?: () => any) =>
fn ? resolvedPromise.then(fn) : resolvedPromise

const scheduler: WatchScheduler = (job, isFirstRun) => {
if (isFirstRun) {
job()
} else {
queue.push(job)
flushJobs()
}
}

const flushJobs = () => {
if (isFlushPending) return
isFlushPending = true
resolvedPromise.then(() => {
queue.forEach(job => job())
queue.length = 0
isFlushPending = false
})
}

describe('watch', () => {
test('effect', () => {
let dummy: any
@@ -147,54 +119,6 @@ describe('watch', () => {
expect(dummy).toBe(30)
})

test('nested calls to baseWatch and onWatcherCleanup', async () => {
let calls: string[] = []
let source: Ref<number>
let copyist: Ref<number>
const scope = new EffectScope()

scope.run(() => {
source = ref(0)
copyist = ref(0)
// sync by default
watch(
() => {
const current = (copyist.value = source.value)
onWatcherCleanup(() => calls.push(`sync ${current}`))
},
null,
{},
)
// with scheduler
watch(
() => {
const current = copyist.value
onWatcherCleanup(() => calls.push(`post ${current}`))
},
null,
{ scheduler },
)
})

await nextTick()
expect(calls).toEqual([])

scope.run(() => source.value++)
expect(calls).toEqual(['sync 0'])
await nextTick()
expect(calls).toEqual(['sync 0', 'post 0'])
calls.length = 0

scope.run(() => source.value++)
expect(calls).toEqual(['sync 1'])
await nextTick()
expect(calls).toEqual(['sync 1', 'post 1'])
calls.length = 0

scope.stop()
expect(calls).toEqual(['sync 2', 'post 2'])
})

test('once option should be ignored by simple watch', async () => {
let dummy: any
const source = ref(0)
7 changes: 3 additions & 4 deletions packages/reactivity/src/arrayInstrumentations.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { isArray } from '@vue/shared'
import { TrackOpTypes } from './constants'
import { ARRAY_ITERATE_KEY, track } from './dep'
import { pauseTracking, resetTracking } from './effect'
import { isProxy, isShallow, toRaw, toReactive } from './reactive'
import { endBatch, startBatch } from './system'
import { endBatch, setActiveSub, startBatch } from './system'

/**
* Track array iteration and return:
@@ -320,10 +319,10 @@ function noTracking(
method: keyof Array<any>,
args: unknown[] = [],
) {
pauseTracking()
startBatch()
const prevSub = setActiveSub()
const res = (toRaw(self) as any)[method].apply(self, args)
setActiveSub(prevSub)
endBatch()
resetTracking()
return res
}
Loading
Oops, something went wrong.