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

Vue3 component can no longer be used directly in another component (v-show) #313

Closed
bobvandevijver opened this issue Jun 4, 2021 · 7 comments

Comments

@bobvandevijver
Copy link

bobvandevijver commented Jun 4, 2021

Describe the bug
When using the <font-awesome-icon> as a direct child in your template, it will throw an error when its state should be changed.

Tagging @otijhuis here has #250 seems to be the root cause.

Reproducible test case
This is my wrapper component, which includes the :key workaround:

<template>
  <font-awesome-icon
    :key="version"
    :fixed-width="!noFixedWidth"
    :icon="icon"
    :spin="spin"
    v-bind="$attrs"/>
</template>

<script lang="ts">
import {defineComponent, ref, watch} from 'vue';

export default defineComponent({
  name: 'SimpleIcon',
  props: {
    icon: {
      type: [String, Array],
      required: true,
    },
    spin: Boolean,
    noFixedWidth: Boolean,
  },
  setup(props) {
    const version = ref(0);
    watch(props, () => version.value++, {deep: true});

    return {
      version,
    };
  },
});
</script>

It triggers the following error whenever I update any prop of the SimpleIcon property:
image

When I change my component to have a wrapper element around the font-awesome-icon, such as a simple span, the error disappears and it works as expected. It also no longer needs the :key hack for the reactivity to work.

Expected behavior
There shouldn't be an error.

Desktop (please complete the following information):

  • Edge 91 (Windows)
  • 3.0.0-4

Additional context
This error didn't happen with 3.0.0-3

@otijhuis
Copy link
Contributor

otijhuis commented Jun 4, 2021

Just added your example component to the test app I used, updated vue to the latest version (3.0.11) and set vue-fontawesome to 3.0.0-4.

I removed :key because it shouldn't be needed anymore and also removed v-bind because that happens automatically when it's the root element. Then I used the SimpleIcon in another component. Toggling spin and changing icon for instance seems to work without any issues. Tried it in firefox, safari and chrome.

So unfortunately I can't reproduce the issue and don't get any errors.

This is what I used:

<template>
  <font-awesome-icon
    :fixed-width="!noFixedWidth"
    :icon="icon"
    :spin="spin"/>
</template>

<script lang="ts">
import {defineComponent} from 'vue';

export default defineComponent({
  name: 'SimpleIcon',
  props: {
    icon: {
      type: [String, Array],
      required: true,
    },
    spin: Boolean,
    noFixedWidth: Boolean,
  },
});
</script>

@bobvandevijver
Copy link
Author

Thank you for trying @otijhuis!

I just found it is triggered by the usage of v-show on the SimpleIcon component, like this:

<SimpleIcon v-show="locale === 'nl'" icon="check"/>

Adding the wrapping span solved it because the v-show was applied on that element, instead of on the icon.

I also confirmed the same error is triggered when using v-show on the font-awesome-icon directly:

<font-awesome-icon v-show="locale === 'nl'" icon="check"/>

The issue does not occur when v-if is used, so could you try to reproduce it with a v-show?

@otijhuis
Copy link
Contributor

otijhuis commented Jun 5, 2021

I can reproduce it now though not with SimpleIcon. Having v-show on SimpleIcon works for me.
When I do the same on font-awesome-icon it fails.

If I'd had to guess the issue is with the last 2 lines of FontAwesomeIcon.js. I hardly ever use render functions and the way I return the function here did feel a bit off since I had to return the value to get it to work. There's probably a better way. Maybe it needs to be wrapped in h(). I believe the SimpleIcon template gets transformed into a render function as well so I'm guessing it wraps this one and that's why it still works.

const vnode = computed(() => renderedIcon.value ? convert(renderedIcon.value.abstract[0], {}, attrs) : null)    
return () => vnode.value

Still curious why v-show wouldn't work though since it only applies CSS styling so I'd expect it to render everything like it normally would and apply the CSS afterwards.

I'll have a look and see if I can fix it.

@otijhuis
Copy link
Contributor

otijhuis commented Jun 5, 2021

Seems that was indeed the issue.

Not sure if it's the best solution but this works:

return () => h(() => vnode.value)

Something like that is probably what happens when font-awesome-icon is wrapped by SimpleIcon.
Tried it in my test application and both SimpleIcon and font-awesome-icon work with v-show now.

FontAwesomeLayersText.js had the same issue as well.

I'll see if I can add a few v-show tests and create a PR with the fix.

EDIT:

Might be best to change the title of the issue to include v-show so it's easier to find.

@otijhuis
Copy link
Contributor

otijhuis commented Jun 5, 2021

I couldn't create a failing test. Maybe the issue was introduced after 3.0.0 which is the current dev dependency.

Created a PR (#315) which should fix the issue.

Maybe you could test the PR by checking out the code from my repo, switch to the 3.x branch, and setting the vue-fontawesome version to "file:../some/location/vue-fontawesome".

@jasonlundien
Copy link
Member

jasonlundien commented Jul 13, 2023

@bobvandevijver & @otijhuis ---
I picked this issue up and was looking into it and just wanted to provide an update (I know it is a couple years old now -- we are trying to clear up issues from the repo).

It seems like the error and warning above are no longer happening when using v-show on the font-awesome-icon itself.
In other words, the following is working:

    <font-awesome-icon
      v-show="locale === 'nl'"
      icon="check"
    />

What is NOT working is when we use v-show with the component that is using the icon.
This is NOT working:

    <SimpleIcon
      v-show="locale === 'nl'"
      icon="check"
    />

Wrapping the above code in a span, div, what have you, will get the icon to work as expected and, "at the moment", the workaround. So this works:

    <span v-show="locale === 'nl'">
      <SimpleIcon icon="check" />
    </span>

As I mentioned above, I am not getting the same warnings or error messages you were getting. I am getting a new "warning" message in the console when using v-show with <SimpleIcon v-show="locale === 'nl'" icon="check" />.

Warning message in console:

image

So at this time, the PR: Fixes toggling v-show fails with icon and layers text #315 will not be popped into master as it does NOT seem to be addressing the warning or the v-show issue when using a component to display the icon in the case of SimpleIcon. I am looking into how we may approach this and will update this this issue again once we something.

@jasonlundien jasonlundien changed the title Vue3 component can no longer be used directly in another component Vue3 component can no longer be used directly in another component (v-show) Jul 13, 2023
@bobvandevijver
Copy link
Author

@jasonlundien Thank you for your effort! I just checked my project, and I can no longer reproduce this issue at all. I'm not even seeing the warning you mention in your post, so I guess this is something Vue itself has fixed. I will close this as magically fixed 😄

@bobvandevijver bobvandevijver closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 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
Development

No branches or pull requests

3 participants