ntohq / buefy-next

Lightweight UI components for Vue.js (Vue3) based on Bulma
https://v3.buefy.org
MIT License
106 stars 10 forks source link

Documentation pages warn Components passed as reactive states #168

Closed kikuomax closed 6 months ago

kikuomax commented 8 months ago

Overview of the problem

Buefy version: [0.1.3-5c420e09933677691b004d437c7c7c684070b972] Vuejs version: [3.3.4] OS/Browser: macOS/Safari

Description

If we open the documentation pages of Buefy-next, we will see a lot of warnings like the following:

[Warning] [Vue warn]: Vue received a Component which was made a reactive object. This can lead to unnecessary performance overhead, and should be avoided by marking the component with `markRaw` or using `shallowRef` instead of `ref`. (31) (chunk-MXVRKIDV.js, line 1393)
"
Component that was made reactive: "
{methods: Object, __hmrId: "cfaa69bd", render: function, __file: "/buefy/packages/doc…src/pages/components/button/examples/ExSimple.vue"}
"
"
" at <Example"
"component="
{methods: Object, __hmrId: "cfaa69bd", render: function, __file: "/buefy/packages/doc…src/pages/components/button/examples/ExSimple.vue"}
"code=\"<template>\\n    <section>\\n        <b-button @click=\\\"clickMe\\\">Click Me</b-button>\\n    </section>\\n</template>\\n\\n<script>\\n    export default {\\n        methods: {\\n            clickMe() {\\n                this.$buefy.notification.open('Clicked!!')\\n            }\\n        }\\n    }\\n</script>\\n\""
"vertical=\"\""
">"
"
"
" at <Button"
"onVnodeUnmounted=fn<onVnodeUnmounted>"
"ref=Ref<"
null
">"
">"
"
"
" at <RouterView>"
"
"
" at <Documentation"
"onVnodeUnmounted=fn<onVnodeUnmounted>"
"ref=Ref<"
Proxy {, …}
">"
">"
"
"
" at <RouterView>"
"
"
" at <Buefy>"
"
"
" at <App>"

Steps to reproduce

  1. Move to packages/docs:

    cd packages/docs
  2. Start a dev server:

    npm run dev
  3. Open http://localhost:5173 with your browser.

  4. Open the JavaScript console on your browser.

  5. Navigate to any component documentation page on your browser.

Expected behavior

No above warnings.

Actual behavior

Got warnings.

wesdevpro commented 7 months ago
[Vue warn]: Vue received a Component which was made a reactive object. This can lead to unnecessary performance overhead, and should be avoided by marking the component with `markRaw` or using `shallowRef` instead of `ref`. 
Component that was made reactive:  {__hmrId: '29c17683', __scopeId: 'data-v-29c17683', __file: 'E:/GitHub/buefy-next/packages/docs/src/pages/components/collapse/examples/ExPosition.vue', render: ƒ} 
  at <Example component= {__hmrId: '29c17683', __scopeId: 'data-v-29c17683', __file: 'E:/GitHub/buefy-next/packages/docs/src/pages/components/collapse/examples/ExPosition.vue', render: ƒ} code="<template>\r\n    <section>\r\n        <div class=\"content\">\r\n            <h3>\r\n                Subtitle\r\n            </h3>\r\n            <p>\r\n                Lorem ipsum dolor sit amet, consectetur adipiscing elit. <br/>\r\n                Nulla accumsan, metus ultrices eleifend gravida, nulla nunc varius lectus, nec rutrum justo nibh eu lectus. <br/>\r\n                Ut vulputate semper dui. Fusce erat odio, sollicitudin vel erat vel, interdum mattis neque.\r\n            </p>\r\n        </div>\r\n        <b-collapse \r\n            :model-value=\"false\"\r\n            position=\"is-bottom\" \r\n            aria-id=\"contentIdForA11y4\">\r\n            <template #trigger=\"props\">\r\n                <a\r\n                    aria-controls=\"contentIdForA11y4\"\r\n                    :aria-expanded=\"props.open\">\r\n                    <b-icon :icon=\"!props.open ? 'menu-down' : 'menu-up'\"></b-icon>\r\n                    {{ !props.open ? 'All options' : 'Fewer options' }}\r\n                </a>\r\n            </template>\r\n            <p>\r\n                Lorem ipsum dolor sit amet, consectetur adipiscing elit. <br/>\r\n                Nulla accumsan, metus ultrices eleifend gravida, nulla nunc varius lectus, nec rutrum justo nibh eu lectus. <br/>\r\n                Ut vulputate semper dui. Fusce erat odio, sollicitudin vel erat vel, interdum mattis neque.\r\n            </p>\r\n        </b-collapse>\r\n    </section>\r\n</template>\r\n\r\n<style scoped>\r\n    .content {\r\n        margin-bottom: 0 !important;\r\n    }\r\n</style>\r\n" title="Position"  ... > 
  at <Collapse onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< undefined > > 
  at <RouterView> 
  at <Documentation onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< undefined > > 
  at <RouterView> 
  at <Buefy> 
  at <App>

@ Example.vue 22

Sources

https://dev.to/bitovi/vuejs-reactivity-system-ref-reactive-shallowref-shallowreactive-oe7 https://stackoverflow.com/questions/70631273/vuejs3-reactivity-of-props-object

wesdevpro commented 7 months ago

@kikuomax image

I was able to resolve the warnings related to the Collapse component by making the Vue Components that are passed as the component prop to the Example.vue component a shallowRef in the Collapse.vue component. However, I don't think this solution is ideal because we would have to manually set every component being passed as the component prop as a shallowRef in the docs. I tried to find a better solution to make the component prop a shallowRef in the Example.vue file without any issues, but nothing seemed to work.

Would you happen to have any recommendations on how to implement shallowRef in the Example? vue component?

kikuomax commented 7 months ago

Would you happen to have any recommendations on how to implement shallowRef in the Example? vue component?

@wesdevpro Few options I came up with:

Since the check happens before Example VNode is created, I do not think Example can do anything to prevent this. See https://github.com/vuejs/core/blob/ee4cd78a06e6aa92b12564e527d131d1064c2cd0/packages/runtime-core/src/vnode.ts#L595-L605

As this is a one-time thing, I think the best way would be to rewrite all the Example usage. You may implement a helper function that reduces the number of lines to change. The function would work like this. Suppose you want to change the following docs page component:

    export default defineComponent({
        data() {
            return {
                api,
                ExSimple,
                ExIcon,
                ExClosable,
                ExTaglist,
                ExTaglistAttached,
                ExFieldCombine,
                ExSizes,
                ExSimpleCode,
                ExIconCode,
                ExClosableCode,
                ExTaglistCode,
                ExTaglistAttachedCode,
                ExFieldCombineCode,
                ExSizesCode
            }
        }
    })

You would use the function, let's say shallowFields, like below:

    export default defineComponent({
        data() {
            return {
                api,
                ...shallowFields({
                    ExSimple,
                    ExIcon,
                    ExClosable,
                    ExTaglist,
                    ExTaglistAttached,
                    ExFieldCombine,
                    ExSizes
                }),
                ExSimpleCode,
                ExIconCode,
                ExClosableCode,
                ExTaglistCode,
                ExTaglistAttachedCode,
                ExFieldCombineCode,
                ExSizesCode
            }
        }
    })

The result should be equivalent to:

    export default defineComponent({
        data() {
            return {
                api,
                ExSimple: shallowRef(ExSimple),
                ExIcon: shallowRef(ExIcon),
                ExClosable: shallowRef(ExClosable),
                ExTaglist: shallowRef(ExTaglist),
                ExTaglistAttached: shallowRef(ExTaglistAttached),
                ExFieldCombine: shallowRef(ExFieldCombine),
                ExSizes: shallowRef(ExSizes),
                ExSimpleCode,
                ExIconCode,
                ExClosableCode,
                ExTaglistCode,
                ExTaglistAttachedCode,
                ExFieldCombineCode,
                ExSizesCode
            }
        }
    })

In my opinion, the code using shallowFields is more legible.

wesdevpro commented 7 months ago

@kikuomax here is what I implemented as a solution based on your recommendations: image image

Let me know if this looks good to you and then I will go ahead and add it to all the components.

kikuomax commented 7 months ago

Let me know if this looks good to you and then I will go ahead and add it to all the components.

@wesdevpro Looks good to me!

Btw, it would become like the following in TypeScript:

import { shallowRef, type ShallowRef } from 'vue'

export function shallowFields<T>(fields: { [K in keyof T]: T[K] }): { [K in keyof T]: ShallowRef<T[K]> } {
    const shallow: { [K in keyof T]: ShallowRef<T[K]> } = {} as any
    for (const key in fields) {
        shallow[key] = shallowRef(fields[key])
    }
    return shallow
}

As in-place updates will unnecessarily complicate typing, it creates a new object with shallowRefed fields.

kikuomax commented 6 months ago

Closed by #183.