-
Notifications
You must be signed in to change notification settings - Fork 667
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: keep the overrides prototype information of component #856
Conversation
This change is actually problematic. We have an issue with components that already been extended (with Vue.extend). They never extend from the localVue class when they are instantiated, so they don't inherit assets like components set in The original code removed by this PR extended all components, including class components, EXCEPT in Vue < 2.3, where components extended with Vue.extend did not merge props correctly. I would like to revert this PR, because it stops us from being able to add assets to the component without mutating it. But I can see that the previous code was causing you issues in your tests. Can you give me some details about why your tests don't work when the component is extended? |
@eddyerburgh Sure. From the implementation of Vue.extend = function (extendOptions: Object): Function {
extendOptions = extendOptions || {}
const Super = this
...
Sub.prototype = Object.create(Super.prototype)
... We can see that if we call
Shall we add some unit test to explain and guarantee this scenario? |
Yes definitely, I made a mistake by merging this PR because no tests were failing. I'd also like to make sure your use case works as expected, but I can't see an easy solution. |
uh... Could you show some code to explain what u wanna achieve? Actually I don't really know yet😂 |
const Constructor = vueVersion < 2.3 && typeof component === 'function'
? component.extend(instanceOptions)
: _Vue.extend(component) |
Could you please provide a test case to describe it? |
it('adds variables to extended components', () => {
const TestComponent = Vue.extend({
template: `
<div>
{{$route.path}}
</div>
`
})
const $route = { path: 'http://test.com' }
const wrapper = mountingMethod(TestComponent, {
mocks: {
$route
}
})
const HTML =
mountingMethod.name === 'renderToString' ? wrapper : wrapper.html()
expect(HTML).contains('http://test.com')
}) |
add the component prototype override scenario and remove the version condition.
@eddyerburgh Actually I don't know why we need add a version condition here: cacfda8 , any special reason?