Skip to content
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(slots): check nullability for multi-branch dynamic slots #6660

Merged
merged 3 commits into from Sep 28, 2022

Conversation

shigma
Copy link
Contributor

@shigma shigma commented Sep 14, 2022

Check nullability for multi-branch dynamic slots, fix ssr regression bug introduced in 3.2.38.

Fix #6639
Fix #6651

@shigma shigma changed the title fix(runtime-core): check nullability for multi-branch dynamic slots fix(slots): check nullability for multi-branch dynamic slots Sep 14, 2022
Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

can you add a unit test?

@shigma shigma requested a review from posva September 25, 2022 11:59
@shigma
Copy link
Contributor Author

shigma commented Sep 25, 2022

Note: I'm not sure the fix touches the the core of problem (although it indeed fixes the regression introduced in 00036bb).

type Slot = (...args: any[]) => VNode[]

From the above typings, a Slot should not return undefined. However it does in the simple reproduction #6639. This means there may be other bugs in the repo but I could not find them (or maybe we need to change the typings).

Despite this, I still think it is necessary to check nullability before we could locate the original source of the problem and even we could, it is still uncertain that there is no more slots returning undefined. After all, this regression will affect all SSR builds which include dynamic slots.

@yyx990803
Copy link
Member

Thanks for the PR, sorry I didn't notice it when pushing 8963c55 - I'm merging this for the unit test.

Regarding the return type - the return value is undefined when the slots are SSR-compiled. In SSR mode the slots have two branches based on whether the child component consuming the slots is also SSR-compiled or vdom-based. The branches are detected based on whether there are SSR-specific helpers being passed in when the slot is called, so this is strictly an internal case and only affect internal runtime helpers that work with slots and are used in both DOM and SSR envs. I've checked the type usage and there are no other helper that falls into this category.

@yyx990803 yyx990803 merged commit 3cc8e02 into vuejs:main Sep 28, 2022
@shigma shigma deleted the fix-ssr-slots branch September 28, 2022 04:11
chrislone pushed a commit to chrislone/core that referenced this pull request Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants