From 7fb6021ac4a97b32ae07f8137801798cdeeb8b55 Mon Sep 17 00:00:00 2001 From: Evan You Date: Sun, 11 Mar 2018 14:30:24 -0400 Subject: [PATCH 1/5] fix: beforeUpdate should be called before render and allow state mutation fix #7481 --- src/core/instance/lifecycle.js | 11 +++++--- src/core/observer/scheduler.js | 3 +++ src/core/observer/watcher.js | 2 ++ .../runtime/components/transition-group.js | 26 +++++++++++-------- test/unit/features/options/lifecycle.spec.js | 15 +++++++++++ 5 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/core/instance/lifecycle.js b/src/core/instance/lifecycle.js index 169a54a6bb3..c5dc7668b98 100644 --- a/src/core/instance/lifecycle.js +++ b/src/core/instance/lifecycle.js @@ -50,9 +50,6 @@ export function initLifecycle (vm: Component) { export function lifecycleMixin (Vue: Class) { Vue.prototype._update = function (vnode: VNode, hydrating?: boolean) { const vm: Component = this - if (vm._isMounted) { - callHook(vm, 'beforeUpdate') - } const prevEl = vm.$el const prevVnode = vm._vnode const prevActiveInstance = activeInstance @@ -197,7 +194,13 @@ export function mountComponent ( // we set this to vm._watcher inside the watcher's constructor // since the watcher's initial patch may call $forceUpdate (e.g. inside child // component's mounted hook), which relies on vm._watcher being already defined - new Watcher(vm, updateComponent, noop, null, true /* isRenderWatcher */) + new Watcher(vm, updateComponent, noop, { + before () { + if (vm._isMounted) { + callHook(vm, 'beforeUpdate') + } + } + }, true /* isRenderWatcher */) hydrating = false // manually mounted instance, call mounted on self diff --git a/src/core/observer/scheduler.js b/src/core/observer/scheduler.js index fce86e5f40b..e7ccf57dd25 100644 --- a/src/core/observer/scheduler.js +++ b/src/core/observer/scheduler.js @@ -53,6 +53,9 @@ function flushSchedulerQueue () { // as we run existing watchers for (index = 0; index < queue.length; index++) { watcher = queue[index] + if (watcher.before) { + watcher.before() + } id = watcher.id has[id] = null watcher.run() diff --git a/src/core/observer/watcher.js b/src/core/observer/watcher.js index 48a2e612e28..3c4e533c08b 100644 --- a/src/core/observer/watcher.js +++ b/src/core/observer/watcher.js @@ -37,6 +37,7 @@ export default class Watcher { newDeps: Array; depIds: SimpleSet; newDepIds: SimpleSet; + before: ?Function; getter: Function; value: any; @@ -58,6 +59,7 @@ export default class Watcher { this.user = !!options.user this.lazy = !!options.lazy this.sync = !!options.sync + this.before = options.before } else { this.deep = this.user = this.lazy = this.sync = false } diff --git a/src/platforms/web/runtime/components/transition-group.js b/src/platforms/web/runtime/components/transition-group.js index 219ff01cf54..81d033d5ad5 100644 --- a/src/platforms/web/runtime/components/transition-group.js +++ b/src/platforms/web/runtime/components/transition-group.js @@ -33,6 +33,21 @@ delete props.mode export default { props, + beforeMount () { + const update = this._update + this._update = (vnode, hydrating) => { + // force removing pass + this.__patch__( + this._vnode, + this.kept, + false, // hydrating + true // removeOnly (!important, avoids unnecessary moves) + ) + this._vnode = this.kept + update.call(this, vnode, hydrating) + } + }, + render (h: Function) { const tag: string = this.tag || this.$vnode.data.tag || 'span' const map: Object = Object.create(null) @@ -76,17 +91,6 @@ export default { return h(tag, null, children) }, - beforeUpdate () { - // force removing pass - this.__patch__( - this._vnode, - this.kept, - false, // hydrating - true // removeOnly (!important, avoids unnecessary moves) - ) - this._vnode = this.kept - }, - updated () { const children: Array = this.prevChildren const moveClass: string = this.moveClass || ((this.name || 'v') + '-move') diff --git a/test/unit/features/options/lifecycle.spec.js b/test/unit/features/options/lifecycle.spec.js index 2b649aa98e4..3fa152e70c2 100644 --- a/test/unit/features/options/lifecycle.spec.js +++ b/test/unit/features/options/lifecycle.spec.js @@ -137,6 +137,21 @@ describe('Options lifecycle hooks', () => { expect(spy).toHaveBeenCalled() }).then(done) }) + + it('should be called before render and allow mutating state', done => { + const vm = new Vue({ + template: '
{{ msg }}
', + data: { msg: 'foo' }, + beforeUpdate () { + this.msg += '!' + } + }).$mount() + expect(vm.$el.textContent).toBe('foo') + vm.msg = 'bar' + waitForUpdate(() => { + expect(vm.$el.textContent).toBe('bar!') + }).then(done) + }) }) describe('updated', () => { From 4f8de9bd67a280b3301fabf42d63382da5bb44c8 Mon Sep 17 00:00:00 2001 From: Evan You Date: Sun, 11 Mar 2018 16:13:03 -0400 Subject: [PATCH 2/5] perf: avoid unnecessary re-renders when computed property value did not change close #7767 --- src/core/instance/state.js | 13 +-- src/core/observer/watcher.js | 101 +++++++++++++------- test/unit/features/options/computed.spec.js | 36 +++++++ test/unit/modules/observer/watcher.spec.js | 54 ++++++++++- 4 files changed, 153 insertions(+), 51 deletions(-) diff --git a/src/core/instance/state.js b/src/core/instance/state.js index b1549b0dcc4..326f7f36bf7 100644 --- a/src/core/instance/state.js +++ b/src/core/instance/state.js @@ -2,7 +2,7 @@ import config from '../config' import Watcher from '../observer/watcher' -import Dep, { pushTarget, popTarget } from '../observer/dep' +import { pushTarget, popTarget } from '../observer/dep' import { isUpdatingChildComponent } from './lifecycle' import { @@ -164,7 +164,7 @@ export function getData (data: Function, vm: Component): any { } } -const computedWatcherOptions = { lazy: true } +const computedWatcherOptions = { computed: true } function initComputed (vm: Component, computed: Object) { // $flow-disable-line @@ -244,13 +244,8 @@ function createComputedGetter (key) { return function computedGetter () { const watcher = this._computedWatchers && this._computedWatchers[key] if (watcher) { - if (watcher.dirty) { - watcher.evaluate() - } - if (Dep.target) { - watcher.depend() - } - return watcher.value + watcher.depend() + return watcher.evaluate() } } } diff --git a/src/core/observer/watcher.js b/src/core/observer/watcher.js index 3c4e533c08b..725480d9215 100644 --- a/src/core/observer/watcher.js +++ b/src/core/observer/watcher.js @@ -29,10 +29,11 @@ export default class Watcher { id: number; deep: boolean; user: boolean; - lazy: boolean; + computed: boolean; sync: boolean; dirty: boolean; active: boolean; + dep: Dep; deps: Array; newDeps: Array; depIds: SimpleSet; @@ -57,16 +58,16 @@ export default class Watcher { if (options) { this.deep = !!options.deep this.user = !!options.user - this.lazy = !!options.lazy + this.computed = !!options.computed this.sync = !!options.sync this.before = options.before } else { - this.deep = this.user = this.lazy = this.sync = false + this.deep = this.user = this.computed = this.sync = false } this.cb = cb this.id = ++uid // uid for batching this.active = true - this.dirty = this.lazy // for lazy watchers + this.dirty = this.computed // for computed watchers this.deps = [] this.newDeps = [] this.depIds = new Set() @@ -89,9 +90,12 @@ export default class Watcher { ) } } - this.value = this.lazy - ? undefined - : this.get() + if (this.computed) { + this.value = undefined + this.dep = new Dep() + } else { + this.value = this.get() + } } /** @@ -162,8 +166,24 @@ export default class Watcher { */ update () { /* istanbul ignore else */ - if (this.lazy) { - this.dirty = true + if (this.computed) { + // A computed property watcher has two modes: lazy and activated. + // It initializes as lazy by default, and only becomes activated when + // it is depended on by at least one subscriber, which is typically + // another computed property or a component's render function. + if (this.dep.subs.length === 0) { + // In lazy mode, we don't want to perform computations until necessary, + // so we simply mark the watcher as dirty. The actual computation is + // performed just-in-time in this.evaluate() when the computed property + // is accessed. + this.dirty = true + } else { + // In activated mode, we want to proactively perform the computation + // but only notify our subscribers when the value has indeed changed. + this.getAndInvoke(() => { + this.dep.notify() + }) + } } else if (this.sync) { this.run() } else { @@ -177,47 +197,54 @@ export default class Watcher { */ run () { if (this.active) { - const value = this.get() - if ( - value !== this.value || - // Deep watchers and watchers on Object/Arrays should fire even - // when the value is the same, because the value may - // have mutated. - isObject(value) || - this.deep - ) { - // set new value - const oldValue = this.value - this.value = value - if (this.user) { - try { - this.cb.call(this.vm, value, oldValue) - } catch (e) { - handleError(e, this.vm, `callback for watcher "${this.expression}"`) - } - } else { - this.cb.call(this.vm, value, oldValue) + this.getAndInvoke(this.cb) + } + } + + getAndInvoke (cb: Function) { + const value = this.get() + if ( + value !== this.value || + // Deep watchers and watchers on Object/Arrays should fire even + // when the value is the same, because the value may + // have mutated. + isObject(value) || + this.deep + ) { + // set new value + const oldValue = this.value + this.value = value + this.dirty = false + if (this.user) { + try { + cb.call(this.vm, value, oldValue) + } catch (e) { + handleError(e, this.vm, `callback for watcher "${this.expression}"`) } + } else { + cb.call(this.vm, value, oldValue) } } } /** - * Evaluate the value of the watcher. - * This only gets called for lazy watchers. + * Evaluate and return the value of the watcher. + * This only gets called for computed property watchers. */ evaluate () { - this.value = this.get() - this.dirty = false + if (this.dirty) { + this.value = this.get() + this.dirty = false + } + return this.value } /** - * Depend on all deps collected by this watcher. + * Depend on this watcher. Only for computed property watchers. */ depend () { - let i = this.deps.length - while (i--) { - this.deps[i].depend() + if (this.dep && Dep.target) { + this.dep.depend() } } diff --git a/test/unit/features/options/computed.spec.js b/test/unit/features/options/computed.spec.js index edc20bad3bf..fc606072e7d 100644 --- a/test/unit/features/options/computed.spec.js +++ b/test/unit/features/options/computed.spec.js @@ -216,4 +216,40 @@ describe('Options computed', () => { }) expect(() => vm.a).toThrowError('rethrow') }) + + // #7767 + it('should avoid unnecessary re-renders', done => { + const computedSpy = jasmine.createSpy('computed') + const updatedSpy = jasmine.createSpy('updated') + const vm = new Vue({ + data: { + msg: 'bar' + }, + computed: { + a () { + computedSpy() + return this.msg !== 'foo' + } + }, + template: `
{{ a }}
`, + updated: updatedSpy + }).$mount() + + expect(vm.$el.textContent).toBe('true') + expect(computedSpy.calls.count()).toBe(1) + expect(updatedSpy.calls.count()).toBe(0) + + vm.msg = 'baz' + waitForUpdate(() => { + expect(vm.$el.textContent).toBe('true') + expect(computedSpy.calls.count()).toBe(2) + expect(updatedSpy.calls.count()).toBe(0) + }).then(() => { + vm.msg = 'foo' + }).then(() => { + expect(vm.$el.textContent).toBe('false') + expect(computedSpy.calls.count()).toBe(3) + expect(updatedSpy.calls.count()).toBe(1) + }).then(done) + }) }) diff --git a/test/unit/modules/observer/watcher.spec.js b/test/unit/modules/observer/watcher.spec.js index 724a3cc8637..ba96b28ffce 100644 --- a/test/unit/modules/observer/watcher.spec.js +++ b/test/unit/modules/observer/watcher.spec.js @@ -144,26 +144,70 @@ describe('Watcher', () => { }).then(done) }) - it('lazy mode', done => { + it('computed mode, lazy', done => { + let getterCallCount = 0 const watcher = new Watcher(vm, function () { + getterCallCount++ return this.a + this.b.d - }, null, { lazy: true }) - expect(watcher.lazy).toBe(true) + }, null, { computed: true }) + + expect(getterCallCount).toBe(0) + expect(watcher.computed).toBe(true) expect(watcher.value).toBeUndefined() expect(watcher.dirty).toBe(true) - watcher.evaluate() + expect(watcher.dep).toBeTruthy() + + const value = watcher.evaluate() + expect(getterCallCount).toBe(1) + expect(value).toBe(5) expect(watcher.value).toBe(5) expect(watcher.dirty).toBe(false) + + // should not get again if not dirty + watcher.evaluate() + expect(getterCallCount).toBe(1) + vm.a = 2 waitForUpdate(() => { + expect(getterCallCount).toBe(1) expect(watcher.value).toBe(5) expect(watcher.dirty).toBe(true) - watcher.evaluate() + + const value = watcher.evaluate() + expect(getterCallCount).toBe(2) + expect(value).toBe(6) expect(watcher.value).toBe(6) expect(watcher.dirty).toBe(false) }).then(done) }) + it('computed mode, activated', done => { + let getterCallCount = 0 + const watcher = new Watcher(vm, function () { + getterCallCount++ + return this.a + this.b.d + }, null, { computed: true }) + + // activate by mocking a subscriber + const subMock = jasmine.createSpyObj('sub', ['update']) + watcher.dep.addSub(subMock) + + const value = watcher.evaluate() + expect(getterCallCount).toBe(1) + expect(value).toBe(5) + + vm.a = 2 + waitForUpdate(() => { + expect(getterCallCount).toBe(2) + expect(subMock.update).toHaveBeenCalled() + + // since already computed, calling evaluate again should not trigger + // getter + watcher.evaluate() + expect(getterCallCount).toBe(2) + }).then(done) + }) + it('teardown', done => { const watcher = new Watcher(vm, 'b.c', spy) watcher.teardown() From 7af77daf7761af7ad5550f300e1750bc667484c1 Mon Sep 17 00:00:00 2001 From: Evan You Date: Sun, 11 Mar 2018 17:09:51 -0400 Subject: [PATCH 3/5] fix(ssr): fix SSR for async functional components fix #7784 --- src/server/render-context.js | 7 ++++- src/server/render.js | 22 +++++++++++++-- test/ssr/ssr-string.spec.js | 54 ++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/server/render-context.js b/src/server/render-context.js index cecb00157d7..b808f6a838d 100644 --- a/src/server/render-context.js +++ b/src/server/render-context.js @@ -67,13 +67,18 @@ export class RenderContext { } switch (lastState.type) { case 'Element': + case 'Fragment': const { children, total } = lastState const rendered = lastState.rendered++ if (rendered < total) { this.renderNode(children[rendered], false, this) } else { this.renderStates.pop() - this.write(lastState.endTag, this.next) + if (lastState.endTag) { + this.write(lastState.endTag, this.next) + } else { + this.next() + } } break case 'Component': diff --git a/src/server/render.js b/src/server/render.js index 150afce077d..c570cc78176 100644 --- a/src/server/render.js +++ b/src/server/render.js @@ -191,7 +191,21 @@ function renderAsyncComponent (node, isRoot, context) { tag ) if (resolvedNode) { - renderComponent(resolvedNode, isRoot, context) + if (resolvedNode.componnetInstance) { + renderComponent(resolvedNode, isRoot, context) + } else if (!Array.isArray(resolvedNode)) { + // single return node from functional component + renderNode(resolvedNode, isRoot, context) + } else { + // multiple return nodes from functional component + context.renderStates.push({ + type: 'Fragment', + children: resolvedNode, + rendered: 0, + total: resolvedNode.length + }) + context.next() + } } else { // invalid component, but this does not throw on the client // so render empty comment node @@ -232,9 +246,10 @@ function renderStringNode (el, context) { const children: Array = el.children context.renderStates.push({ type: 'Element', + children, rendered: 0, total: children.length, - endTag: el.close, children + endTag: el.close }) write(el.open, next) } @@ -263,9 +278,10 @@ function renderElement (el, isRoot, context) { const children: Array = el.children context.renderStates.push({ type: 'Element', + children, rendered: 0, total: children.length, - endTag, children + endTag }) write(startTag, next) } diff --git a/test/ssr/ssr-string.spec.js b/test/ssr/ssr-string.spec.js index f370618d8bc..4d3ace46019 100644 --- a/test/ssr/ssr-string.spec.js +++ b/test/ssr/ssr-string.spec.js @@ -575,6 +575,60 @@ describe('SSR: renderToString', () => { }) }) + it('renders async component (functional, single node)', done => { + renderVmWithOptions({ + template: ` +
+ +
+ `, + components: { + testAsync (resolve) { + setTimeout(() => resolve({ + functional: true, + render (h) { + return h('span', { class: ['b'] }, 'testAsync') + } + }), 1) + } + } + }, result => { + expect(result).toContain('
testAsync
') + done() + }) + }) + + it('renders async component (functional, multiple nodes)', done => { + renderVmWithOptions({ + template: ` +
+ +
+ `, + components: { + testAsync (resolve) { + setTimeout(() => resolve({ + functional: true, + render (h) { + return [ + h('span', { class: ['a'] }, 'foo'), + h('span', { class: ['b'] }, 'bar') + ] + } + }), 1) + } + } + }, result => { + expect(result).toContain( + '
' + + 'foo' + + 'bar' + + '
' + ) + done() + }) + }) + it('should catch async component error', done => { Vue.config.silent = true renderToString(new Vue({ From 31fd08f292bca86258f27f5416f9e239ba3ce513 Mon Sep 17 00:00:00 2001 From: javoski Date: Tue, 1 May 2018 18:54:36 +0800 Subject: [PATCH 4/5] fix(compiler): maybeComponent should aware of "is" attribute --- src/compiler/codegen/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/codegen/index.js b/src/compiler/codegen/index.js index 1c912c4d96f..b07408124dc 100644 --- a/src/compiler/codegen/index.js +++ b/src/compiler/codegen/index.js @@ -26,7 +26,7 @@ export class CodegenState { this.dataGenFns = pluckModuleFunction(options.modules, 'genData') this.directives = extend(extend({}, baseDirectives), options.directives) const isReservedTag = options.isReservedTag || no - this.maybeComponent = (el: ASTElement) => !isReservedTag(el.tag) + this.maybeComponent = (el: ASTElement) => !(isReservedTag(el.tag) && !el.component) this.onceId = 0 this.staticRenderFns = [] } From 5402f27a894aed7e3859698e829b472effa626d2 Mon Sep 17 00:00:00 2001 From: javoski Date: Tue, 1 May 2018 18:55:08 +0800 Subject: [PATCH 5/5] add test case --- test/unit/modules/compiler/codegen.spec.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/modules/compiler/codegen.spec.js b/test/unit/modules/compiler/codegen.spec.js index 93a27f3b245..126453555f1 100644 --- a/test/unit/modules/compiler/codegen.spec.js +++ b/test/unit/modules/compiler/codegen.spec.js @@ -524,6 +524,11 @@ describe('codegen', () => { '
', `with(this){return _c(component1,{tag:"div"})}` ) + // maybe a component and normalize type should be 1 + assertCodegen( + '
', + `with(this){return _c('div',[_c("component1",{tag:"div"})],1)}` + ) }) it('generate component with inline-template', () => {