VwcMenu - onAction does not work after version `2.8.7`

sima-vonage commented 2 years ago

After upgrading vivid-react to any version higher than 2.8.7, for example v2.8.11 , the onAction of VwcMenu does not work anymore.

Please let me know if you require any more information.

k-paxian commented 2 years ago

Well I've checked those with the latest version, even added a storybook story to test it out here seems working fine for me. Could you please specify more details on your setup?

andrii-vonage commented 2 years ago

Hey @k-paxian please check out this screen record with the issue, hope it helps to identify it.

k-paxian commented 2 years ago

From what I've got so far:

Context 1:

https://github.com/nexmoinc/neru-frontend App is built/assembled using react-scripts :atom:

This is how looks like working 🟢 menu component "@vonage/vwc-list": "2.18.1" shipped alongside "@vonage/vivid-react": "2.8.7"

This is how looks like broken 🔴 menu component "@vonage/vwc-list": "2.20.1" shipped alongside "@vonage/vivid-react": "2.8.12"

Context 2:

https://github.com/Vonage/vivid-react https://github.com/Vonage/vivid-react/blob/master/stories/VwcMenu/VwcMenu.stories.jsx App is built/assembled using storybook 📖

This is how looks like working 🟢 menu component "@vonage/vwc-list": "2.20.1" shipped alongside "@vonage/vivid-react": "2.8.12"

Menu component is working fine 🤷

Context 3:

https://vivid.vonage.com https://vivid.vonage.com/?path=/story/components-menu--with-vwc-list-item App is built/assembled using storybook 📖 Menu component is working fine 🤷

Analytics

The only difference between contexts: the tool which is used to build the apps react-scripts vs storybook we at VCC are using custom webpack configs and this menu component works well for us 💥

This case definitely needs further investigation, to find the exact root cause 😄 Somehow newest menu component version conflicts with the react-scripts way of building the web app.

My bet is that react-scripts has embedded webpack config which is not quite suitable enough to the task, and taking into account different menu items height, I would assume that config is doing something terrible with the material web component styles 😉

andrii-vonage commented 2 years ago

Thanks for your investigation @k-paxian. The earliest version the bug was discovered is 2.8.9, perhaps narrowing down to the changes, that were made between 2.8.7 and 2.8.9 can shed light on it?

Reverting back to 2.8.7 doesn't solve the problem as internal packages are not reverted to exact version, this makes me think that the cause is rooted in one of its dependencies.

Off-topic: did you consider pinning dependencies versions?

andrii-vonage commented 2 years ago

Apparently this happens because VwcList is not eagerly imported together with VwcMenu although it's used by it. Importing it explicitly makes it work again Ⓒ (as per @tveinfeld suggestion), but this is considered a bug. @yinonov is going to take care of it.

k-paxian commented 2 years ago

Wow, I've missed that case 😞. We could fix it on vivid-react side as well. When we are importing atom element we should eagerly import host element package as well. Since we know that both elements lives inside one package. i.e. VwcList is shipped alongside VwcListItem from a single package "@vonage/vwc-list".

Hope we could tide it up from both ends to prevent such cases in future 😄

andrii-vonage commented 2 years ago

@yinonov just released 2.22.0 with the fix. I just reinstalled previously broken 2.8.12 version of vivid-react and it just works. It's cool, but it doesn't seem right. 🙂 I think the versions range should be stricter @k-paxian

k-paxian commented 2 years ago

@andrii-vonage Makes a lot of sense to me indeed, since v2.8.14 dependencies are pinned to exact versions. Also, please let me know if this resolves the issue completely. Thank you so much for active participation in making it better 😉

andrii-vonage commented 2 years ago

That was surprisingly fast! I can confirm that 2.8.14 works perfectly, thanks @k-paxian! 👍

Vonage / vivid-react