fix: improve slots option #813
Conversation
| const vnode = h(el) | ||
| let vnode = h(el) | ||
| if (typeof slotValue === 'string') { | ||
| const vue = new Vue() |
There was a problem hiding this comment.
I think this would be a problem if you mount a component that uses localVue and the slot component accesses a property that is only added to the localVue prototype, can you write a test to check?
There was a problem hiding this comment.
Yes, it is.
Changing localVue.prototype, this does not work.
Using vm._renderProxy is correct if possible.
I will add test for changing localVue.prototype.
|
This case does not work. slots: {
default: '<div><component-with-parent-name /></div>'
} |
|
I added a test for changing |
| @@ -22,6 +23,12 @@ function createVNodesForSlot ( | |||
| let vnode = h(el) | |||
| if (typeof slotValue === 'string') { | |||
| const vue = new Vue() | |||
| const _vue = new _Vue() | |||
There was a problem hiding this comment.
why not just use the _vue.renderProxy rather than mixing both?
There was a problem hiding this comment.
Since it raises an error.
|
@eddyerburgh |
|
I think #821 is better than this. |
| const _vue = new _Vue() | ||
| for (const key in _vue._renderProxy) { | ||
| if (!(vue._renderProxy[key])) { | ||
| vue._renderProxy[key] = _vue._renderProxy[key] |
There was a problem hiding this comment.
Why not always use _vue.renderProxy?
There was a problem hiding this comment.
When vue.renderProxy is used, it raises an error.
When _vue.renderProxy is used, it raises an error.
TypeError: Cannot read property 'Ctor' of undefined
There was a problem hiding this comment.
Do you mean when _vue.renderProxy is used?
There was a problem hiding this comment.
I am sorry.
Yes, I do.
| } | ||
| try { | ||
| // $FlowIgnore | ||
| vnode = el.render.call(vue._renderProxy, h) |
There was a problem hiding this comment.
Do components without a render function work here?
For example:
mount(TestComponent,{
slots: { default: {
template: '<div />'
}}
})Can you add a test for this kind of slot
|
Could you also include a fix for #828? Your previous PR would have solved this (adding a parent div to template strings, and using the child value) |
|
I will try to add it. |
|
@eddyerburgh |
|
Ok sounds good. Can you add the tests from #734 to verify that this PR solves the issue? |
|
@eddyerburgh |
|
Nice work, thanks :) |
|
@eddyerburgh |
This resolves #793.