Skip to content

Conversation

posva
Copy link
Member

@posva posva commented Jun 16, 2021

The actual use case comes from Vue Router when doing:

<router-view v-slot="{ Component }">
    <transition mode="out-in">
      <keep-alive>
        <suspense>
          <component :is="Component"></component>
          <template #fallback>
            <div>
              Loading...
            </div>
          </template>
        </suspense>
      </keep-alive>
    </transition>
</router-view>

Component will be null initially (initial navigation) or when nothing is matched. The warning could be avoided by using <component v-if="Component" :is="Component" /> but I think having the warn could be confusing for users as they would be looking for a multi-node root they don't have

I also added missing tests for the existing warning

@posva posva added scope: suspense 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. labels Jun 16, 2021
@@ -26,7 +26,7 @@ export function resolveComponent(
return resolveAsset(COMPONENTS, name, true, maybeSelfReference) || name
}

export const NULL_DYNAMIC_COMPONENT = Symbol()
export const NULL_DYNAMIC_COMPONENT = Symbol(__DEV__ ? 'Null' : undefined)
Copy link
Member Author

Choose a reason for hiding this comment

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

I did this to make it Similar to Text, Static, and other existing symbols in vnode.ts. It could also be moved with the other existing vnodes

@github-actions
Copy link

github-actions bot commented Jun 16, 2021

Size report

Path Size
vue.global.prod.js 42.56 KB (+0.01% 🔺)
runtime-dom.global.prod.js 28.16 KB (+0.04% 🔺)
size-check.global.prod.js 16.91 KB (+0.04% 🔺)

@@ -104,6 +104,7 @@ export type VNodeProps = {

type VNodeChildAtom =
| VNode
| typeof NULL_DYNAMIC_COMPONENT
Copy link
Member Author

Choose a reason for hiding this comment

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

This turns out to be possible so I added it

@jods4
Copy link
Contributor

jods4 commented Aug 10, 2021

Would be nice to merge this PR if it's ready, or maybe keep the related issue #4016 open.
It was closed more than one month ago but it's not fixed in every release that happened since then.

@MisterDr
Copy link

Could you bump this one? So far this warning generated probably a few terabytes of logs.

@posva posva force-pushed the fix/avoid-warn-empty-suspense branch from e09f847 to ed587e3 Compare October 31, 2023 16:09
@posva posva added the ready to merge The PR is ready to be merged. label Oct 31, 2023
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 86.5 kB 32.9 kB (+6 B) 29.6 kB (-67 B)
vue.global.prod.js 132 kB 49.6 kB (+1 B) 44.5 kB (+10 B)

Usages

Name Size Gzip Brotli
createApp 48 kB (+6 B) 18.9 kB (-2 B) 17.2 kB
createSSRApp 51.2 kB (+6 B) 20.2 kB (-3 B) 18.4 kB (-2 B)
defineCustomElement 50.3 kB (+6 B) 19.7 kB (-6 B) 17.9 kB (+8 B)
overall 61.3 kB 23.7 kB (+3 B) 21.6 kB (+2 B)

@yyx990803 yyx990803 merged commit 405f345 into main Nov 6, 2023
@yyx990803 yyx990803 deleted the fix/avoid-warn-empty-suspense branch November 6, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. ready to merge The PR is ready to be merged. scope: suspense
Projects
Development

Successfully merging this pull request may close these issues.

6 participants