-
Notifications
You must be signed in to change notification settings - Fork 669
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: improve slots mounting option #821
Conversation
I will add some tests and improvements within 3 days. |
955689d
to
4fc7323
Compare
962d5ef
to
da3e193
Compare
slotValue: Component | string | ||
): ?Array<VNode> { | ||
if (typeof slotValue === 'string') { | ||
const compiledResult = compileToFunctions(`<div>${slotValue}{{ }}</div>`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the empty interpolation {{ }}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand.
It is not necessary.
I thought the compileToFunctions raises an error if it dose not exist at #274.
I think it is necessary to add warning if |
// it is not necessary to check compileToFunctions. | ||
const compiledResult = compileToFunctions(`<div>${slotValue}</div>`) | ||
const _staticRenderFns = vm._renderProxy.$options.staticRenderFns | ||
vm._renderProxy.$options.staticRenderFns = compiledResult.staticRenderFns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you save the renderFunctions and then reassign?
() => { | ||
const wrapper1 = mount(ComponentWithSlots, { slots: { default: 'foo<span>123</span>{{ foo }}' }}) | ||
expect(wrapper1.find('main').html()).to.equal('<main>foo<span>123</span>bar</main>') | ||
const wrapper2 = mount(ComponentWithSlots, { slots: { default: '<p>1</p>{{ foo }}2' }}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is how slots should behave. foo should not be resolved using the child vm that it's rendered in. If it uses any instance to resolve foo on, it should be the root instance. But I don't think we should support any variables in the slots option as it adds too much complexity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry.
Your pointing out is correct.
I misunderstood about it.
return [vm.$createElement(slotValue)] | ||
} | ||
|
||
export default function createRenderSlot ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of replacing the renderSlot alias? Does this solve the issue of parent being undefined?
This resolves #793.
This is related to #813.
This does not reslove #827.