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

Icon prop is not reactive #1

Closed
andrey-hohlov opened this issue Jun 4, 2021 · 3 comments
Closed

Icon prop is not reactive #1

andrey-hohlov opened this issue Jun 4, 2021 · 3 comments

Comments

@andrey-hohlov
Copy link

Hi,

Let's say we have a template like this

<SvgVue :icon="icon" />

where the icon is a dynamic value.

Now only the initial value applies.

Works with these changes:

const iconPath = computed(() => props.icon.replace(new RegExp('.'.replace(/([.*+?^=!:${}()|\[\]\/\\])/g, '\\$1'), 'g'), '/') + '.svg');

const svgString = computed(() => require(`svg-files-path/${iconPath.value}`).default);
const svgViewBoxValues = computed(() => svgString ? (/viewBox="([^"]+)"/.exec(svgString.value) || '')[1] : null);
const svgContent = computed(() => svgString ? svgString.value.replace(/^<svg[^>]*>|<\/svg>$/g, '') : null);

https://github.com/danielstgt/svg-vue3/blob/master/src/svg-vue.vue#L21

Of course, there is a workaround:

<SvgVue :icon="icon" :key="icon" />

But can you explain, please, if there are any reasons (performance-wise?) to not make it reactive?

The same for the component for Vue 2, I use both on different projects.

@danielstgt
Copy link
Owner

Hey,

the reason the icon ist not reactive is due to Vue's diffing - components are "cached" without a key. This isn't something that is exclusive to SvgVue, it applies to every dynamic component.

From the Vue docs:

The key special attribute is primarily used as a hint for Vue’s virtual DOM algorithm to identify VNodes when diffing the new list of nodes against the old list. Without keys, Vue uses an algorithm that minimizes element movement and tries to patch/reuse elements of the same type in-place as much as possible. With keys, it will reorder elements based on the order change of keys, and elements with keys that are no longer present will always be removed/destroyed.

Children of the same common parent must have unique keys. Duplicate keys will cause render errors.

Source: https://vuejs.org/v2/api/#key

The way to go here is adding a uniqueue key attribute, this is also necessary if you are toggling icons (which is actually happening with dynamic icon values). A note about that can be found here: Toggling Icons

A related issue to this topic is also here: danielstgt/laravel-mix-svg-vue#3 (comment)

I hope this answers your question, if not please reopen this issue and let me know if there is anything unclear.

@andrey-hohlov
Copy link
Author

andrey-hohlov commented Jun 5, 2021

@danielstgt thanks for the answer.

Maybe my message was unclear.
I know about the key attribute and I use it as a workaround.

I don't switch SvgVue components like

<SvgVue v-if="..." icon="..."/>
<SvgVue v-else="..." icon="..."/>

but just change SvgVue component's icon prop. This change doesn't switch the icon.

<script>
export default {
  data() {
    return {
      icon: 'success',
    };
  },
  methods: {
    toggleIcon() {
      this.icon = this.icon === 'success' ? 'error' : 'success';
    },
  },
};
</script>


<template>
  <div>
    <button type="button" @click="toggleIcon()" />
    <SvgVue :icon="icon"/>
  </div>
</template>

As you can see above, I reach the reactivity of this prop with some component code changes and it works.

So, it's not about Vue reactivity, but about the component realization.

I supposed maybe you realized some performance caveats in making property reactive. But maybe you just missed this part?

@danielstgt
Copy link
Owner

Oh, now I've got it!

I actually misunderstood your initial issue a bit, but I was able to recreate your case — you are right, there is actually no reason why the iconPath wasn't computed.

I've build the following component:

<template>
    <div>
        <svg-vue :icon="selectedIconName" class="w-10 h-10"></svg-vue>

        <div>
            <button @click="changeIcon('error')" class="border rounded">Change Icon to Error</button>
            <button @click="changeIcon('success')" class="ml-4 border rounded">Change Icon to Success</button>
        </div>
    </div>
</template>

<script>
import { ref } from 'vue';

export default {
    setup() {
        let selectedIconName = ref('success');
        let changeIcon = (newIcon) => selectedIconName.value = newIcon;

        return { selectedIconName, changeIcon };
    }
}
</script>

And the equivalent version for Vue 2:

<template>
    <div>
        <svg-vue :icon="selectedIconName" class="w-10 h-10"></svg-vue>

        <div>
            <button @click="changeIcon('error')" class="border rounded">Change Icon to Error</button>
            <button @click="changeIcon('success')" class="ml-4 border rounded">Change Icon to Success</button>
        </div>
    </div>
</template>

<script>
export default {
    data() {
        return {
            selectedIconName: 'success',
        }
    },

    methods: {
        changeIcon(newIcon) {
            this.selectedIconName = newIcon;
        }
    }
}
</script>

Initially they didn't work. However, I've upgraded the svg-vue and svg-vue3 components with your suggested changes. I couldn't see any performance drawbacks and they both work now.

The reason why I never encountered this problem was the use of key attributes in my projects...

A new version of the laravel-mix-svg-vue plugin is available, which has the upgraded dependencies: https://github.com/danielstgt/laravel-mix-svg-vue/releases/tag/v0.3.3

Thank you 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants