Skip to content

fix: reconcile the overridden prototype of component and _Vue mocks and mixins #912

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: reconcile the overridden prototype of component and _Vue mocks and mixins #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 29 additions & 3 deletions packages/create-instance/create-instance.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow

import Vue from 'vue'
import { createSlotVNodes } from './create-slot-vnodes'
import addMocks from './add-mocks'
import { addEventLogger } from './log-events'
Expand Down Expand Up @@ -109,10 +110,35 @@ export default function createInstance (
component.options._base = _Vue
}

function getRootVueProto (obj) {
while (obj) {
if (Object.getPrototypeOf(obj) === Vue.prototype) {
return obj
}

obj = Object.getPrototypeOf(obj)
}
}

function getExtendedComponent (component, instanceOptions) {
// extend _Vue to merge the mixins on _Vue
const extendedComponent = component.extend(_Vue).extend(instanceOptions)

// cache subclass constructor
component.options._Ctor[extendedComponent.cid] = extendedComponent

// to keep the possible overridden prototype and _Vue mocks on prototype,
// we need change the proto chains manually
// @see https://github.com/vuejs/vue-test-utils/pull/856
const root = getRootVueProto(extendedComponent.prototype)
Object.setPrototypeOf(root, _Vue.prototype)
Copy link
Member

Choose a reason for hiding this comment

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

I believe this would mutate components:

const CompA = Vue.extend()
const CompB = CompA.extend().extend()
const savedProto = CompA.prototype.__proto__
const rootProto = CompB.prototype.__proto__.__proto__
Object.setPrototypeOf(rootProto, null)

console.log(savedProto === CompA.prototype.__proto__) // false

Copy link
Contributor Author

@kuitos kuitos Aug 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

yes it could if you set the proto of rootProto to be null.
But here we actually did was Object.setPrototypeOf(rootProto, _Vue.prototype)

const rootProto = CompB.prototype.__proto__.__proto__
Object.setPrototypeOf(rootProto, Vue.prototype)

console.log(savedProto === CompA.prototype.__proto__) // true

Copy link
Member

@eddyerburgh eddyerburgh Aug 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If you set it to _Vue then I think it would still mutate the protoype, it just wouldn't be as severe:

const CompA = Vue.extend()
const CompB = CompA.extend().extend()
const _Vue = Vue.extend()

const savedProto = CompA.prototype.__proto__
const rootProto = CompB.prototype.__proto__.__proto__
Object.setPrototypeOf(rootProto, _Vue.prototype)

console.log(savedProto === CompA.prototype.__proto__) // false

Copy link
Contributor Author

@kuitos kuitos Aug 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with what you concerned about, I will add an unit test and find a way to guarantee the super component immutably.
What do you think? Any suggestion? :)

Copy link
Member

Choose a reason for hiding this comment

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

That would be great, I don't have any suggestions unfortunately. I spent some time two weeks ago investigating, but I couldn't find a solution that didn't mutate the original components


return extendedComponent
}

// extend component from _Vue to add properties and mixins
// extend does not work correctly for sub class components in Vue < 2.2
const Constructor = typeof component === 'function' && vueVersion < 2.3
? component.extend(instanceOptions)
const Constructor = typeof component === 'function'
? getExtendedComponent(component, instanceOptions)
: _Vue.extend(component).extend(instanceOptions)

// Keep reference to component mount was called with
Expand Down
42 changes: 22 additions & 20 deletions test/specs/mount.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,27 +156,29 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => {
expect(stub).not.called
})

it.skip('overrides component prototype', () => {
const mountSpy = sinon.spy()
const destroySpy = sinon.spy()
const Component = Vue.extend({})
const { $mount: originalMount, $destroy: originalDestroy } = Component.prototype
Component.prototype.$mount = function (...args) {
originalMount.apply(this, args)
mountSpy()
return this
}
Component.prototype.$destroy = function () {
originalDestroy.apply(this)
destroySpy()
}
itDoNotRunIf(
vueVersion < 2.3,
'overrides component prototype', () => {
const mountSpy = sinon.spy()
const destroySpy = sinon.spy()
const Component = Vue.extend({})
const { $mount: originalMount, $destroy: originalDestroy } = Component.prototype
Component.prototype.$mount = function (...args) {
originalMount.apply(this, args)
mountSpy()
return this
}
Component.prototype.$destroy = function () {
originalDestroy.apply(this)
destroySpy()
}

const wrapper = mount(Component)
expect(mountSpy).called
expect(destroySpy).not.called
wrapper.destroy()
expect(destroySpy).called
})
const wrapper = mount(Component)
expect(mountSpy).called
expect(destroySpy).not.called
wrapper.destroy()
expect(destroySpy).called
})

// Problems accessing options of twice extended components in Vue < 2.3
itDoNotRunIf(vueVersion < 2.3, 'compiles extended components', () => {
Expand Down